[flang-commits] [flang] 9fc79ae - [flang] Move main variables static promotion from lowering into IsSaved
Jean Perier via flang-commits
flang-commits at lists.llvm.org
Tue Sep 27 23:40:02 PDT 2022
Author: Jean Perier
Date: 2022-09-28T08:37:22+02:00
New Revision: 9fc79ae628d38559803c63f61fc6a8310b909632
URL: https://github.com/llvm/llvm-project/commit/9fc79ae628d38559803c63f61fc6a8310b909632
DIFF: https://github.com/llvm/llvm-project/commit/9fc79ae628d38559803c63f61fc6a8310b909632.diff
LOG: [flang] Move main variables static promotion from lowering into IsSaved
Currently, lowering is promoting main program array and character
variables that are not saved into static memory.
This causes issues with equivalence initial value images because
semantics is relying on IsSaved to build the initial value of variables
in static memory. It seems more robust to have IsSaved be the place
deciding if a variable needs to be in static memory (except for common
block members).
Move the logic to decide if a main program variable must be in static
memory into evaluate::IsSaved and add two options to semantics to
replace the llvm options that were used in lowering:
- SaveMainProgram (off by default): save all main program variables.
- SaveBigMainProgramVariables (on by default): save all main program
variables that are bigger than 32 bytes.
The first options is required to run a few old programs that expect all
main program variables to be in bss (and therefore zero initialized).
The second option is added to allow performance testing: placing big
arrays in static memory seems a sane default to avoid blowing up the
stack with old programs that define big local arrays in the main
program, but since it is easier to prove that an alloca does not
escape/is not modified by calls, keeping big arrays on the stack could
yield improvements.
The logic of SaveBigMainProgramVariables is slightly changed compared to what
it was doing in lowering. The old code was placing all arrays and all
explicit length characters in static memory.
The new code is placing everything bigger than 32 bytes in static
memory. This has the advantages of being a simpler logic, and covering
the cases of scalar derived type with big array components or many
components. Small strings and arrays are now left on the stack (after
all, a character(1) can fit in register).
Note: I think it could have been nicer to add a single "integer" option
to set a threshold to place main program variables in static memory so
that this can be fine tuned by the drivers (SaveMainProgram would be
implemented by setting it to zero). But the language feature options are
not meant to carry integer options. Extending it for this seems an
overkill precedent, and placing it in SemanticsContext is weird (it is
a too low level option to be a bare member of SemanticsContext in my
opinion). So I just rolled my own dices and picked 32 for the sake of
simplicity.
Differential Revision: https://reviews.llvm.org/D134735
Added:
Modified:
flang/include/flang/Common/Fortran-features.h
flang/lib/Evaluate/tools.cpp
flang/lib/Lower/PFTBuilder.cpp
flang/test/Lower/OpenMP/atomic-read.f90
flang/test/Lower/array-character.f90
flang/test/Lower/array-expression-slice-1.f90
flang/test/Lower/associate-construct.f90
flang/test/Lower/io-write.f90
flang/test/Lower/namelist.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Common/Fortran-features.h b/flang/include/flang/Common/Fortran-features.h
index 2411e7cd0293..b94cf2c6d21f 100644
--- a/flang/include/flang/Common/Fortran-features.h
+++ b/flang/include/flang/Common/Fortran-features.h
@@ -32,7 +32,8 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
OldLabelDoEndStatements, LogicalIntegerAssignment, EmptySourceFile,
ProgramReturn, ImplicitNoneTypeNever, ImplicitNoneTypeAlways,
ForwardRefDummyImplicitNone, OpenAccessAppend, BOZAsDefaultInteger,
- DistinguishableSpecifics, DefaultSave, PointerInSeqType, NonCharacterFormat)
+ DistinguishableSpecifics, DefaultSave, PointerInSeqType, NonCharacterFormat,
+ SaveMainProgram, SaveBigMainProgramVariables)
using LanguageFeatures = EnumSet<LanguageFeature, LanguageFeature_enumSize>;
@@ -46,6 +47,7 @@ class LanguageFeatureControl {
disable_.set(LanguageFeature::ImplicitNoneTypeNever);
disable_.set(LanguageFeature::ImplicitNoneTypeAlways);
disable_.set(LanguageFeature::DefaultSave);
+ disable_.set(LanguageFeature::SaveMainProgram);
// These features, if enabled, conflict with valid standard usage,
// so there are disabled here by default.
disable_.set(LanguageFeature::BackslashEscapes);
diff --git a/flang/lib/Evaluate/tools.cpp b/flang/lib/Evaluate/tools.cpp
index 51b09f3b7af4..31f9d5c1331e 100644
--- a/flang/lib/Evaluate/tools.cpp
+++ b/flang/lib/Evaluate/tools.cpp
@@ -1403,6 +1403,8 @@ bool IsAutomatic(const Symbol &original) {
bool IsSaved(const Symbol &original) {
const Symbol &symbol{GetAssociationRoot(original)};
const Scope &scope{symbol.owner()};
+ const common::LanguageFeatureControl &features{
+ scope.context().languageFeatures()};
auto scopeKind{scope.kind()};
if (symbol.has<AssocEntityDetails>()) {
return false; // ASSOCIATE(non-variable)
@@ -1422,8 +1424,18 @@ bool IsSaved(const Symbol &original) {
// BLOCK DATA entities must all be in COMMON,
// which was checked above.
return true;
- } else if (scope.context().languageFeatures().IsEnabled(
- common::LanguageFeature::DefaultSave) &&
+ } else if (scopeKind == Scope::Kind::MainProgram &&
+ (features.IsEnabled(common::LanguageFeature::SaveMainProgram) ||
+ (features.IsEnabled(
+ common::LanguageFeature::SaveBigMainProgramVariables) &&
+ symbol.size() > 32))) {
+ // With SaveBigMainProgramVariables, keeping all unsaved main program
+ // variables of 32 bytes or less on the stack allows keeping numerical and
+ // logical scalars, small scalar characters or derived, small arrays, and
+ // scalar descriptors on the stack. This leaves more room for lower level
+ // optimizers to do register promotion or get easy aliasing information.
+ return true;
+ } else if (features.IsEnabled(common::LanguageFeature::DefaultSave) &&
(scopeKind == Scope::Kind::MainProgram ||
(scope.kind() == Scope::Kind::Subprogram &&
!(scope.symbol() &&
diff --git a/flang/lib/Lower/PFTBuilder.cpp b/flang/lib/Lower/PFTBuilder.cpp
index 59a9b2cdcb67..1c93fd4ba9aa 100644
--- a/flang/lib/Lower/PFTBuilder.cpp
+++ b/flang/lib/Lower/PFTBuilder.cpp
@@ -24,19 +24,6 @@ static llvm::cl::opt<bool> clDisableStructuredFir(
"no-structured-fir", llvm::cl::desc("disable generation of structured FIR"),
llvm::cl::init(false), llvm::cl::Hidden);
-static llvm::cl::opt<bool> nonRecursiveProcedures(
- "non-recursive-procedures",
- llvm::cl::desc("Make procedures non-recursive by default. This was the "
- "default for all Fortran standards prior to 2018."),
- llvm::cl::init(/*2018 standard=*/false));
-
-static llvm::cl::opt<bool> mainProgramGlobals(
- "main-program-globals",
- llvm::cl::desc(
- "Allocate all variables in the main program as global variables and "
- "not on the stack regardless of type, kind, and rank."),
- llvm::cl::init(/*2018 standard=*/false), llvm::cl::Hidden);
-
using namespace Fortran;
namespace {
@@ -1268,40 +1255,8 @@ bool Fortran::lower::definedInCommonBlock(const semantics::Symbol &sym) {
return semantics::FindCommonBlockContaining(sym);
}
-static bool isReentrant(const Fortran::semantics::Scope &scope) {
- if (scope.kind() == Fortran::semantics::Scope::Kind::MainProgram)
- return false;
- if (scope.kind() == Fortran::semantics::Scope::Kind::Subprogram) {
- const Fortran::semantics::Symbol *sym = scope.symbol();
- assert(sym && "Subprogram scope must have a symbol");
- return sym->attrs().test(semantics::Attr::RECURSIVE) ||
- (!sym->attrs().test(semantics::Attr::NON_RECURSIVE) &&
- Fortran::lower::defaultRecursiveFunctionSetting());
- }
- if (scope.kind() == Fortran::semantics::Scope::Kind::Module)
- return false;
- return true;
-}
-
/// Is the symbol `sym` a global?
bool Fortran::lower::symbolIsGlobal(const semantics::Symbol &sym) {
- if (const auto *details = sym.detailsIf<semantics::ObjectEntityDetails>()) {
- if (details->init())
- return true;
- if (!isReentrant(sym.owner())) {
- // Turn array and character of non re-entrant programs (like the main
- // program) into global memory.
- if (const Fortran::semantics::DeclTypeSpec *symTy = sym.GetType())
- if (symTy->category() == semantics::DeclTypeSpec::Character)
- if (auto e = symTy->characterTypeSpec().length().GetExplicit())
- return true;
- if (!details->shape().empty() || !details->coshape().empty())
- return true;
- }
- if (mainProgramGlobals &&
- sym.owner().kind() == Fortran::semantics::Scope::Kind::MainProgram)
- return true;
- }
return semantics::IsSaved(sym) || lower::definedInCommonBlock(sym) ||
semantics::IsNamedConstant(sym);
}
@@ -1695,14 +1650,6 @@ Fortran::lower::createPFT(const parser::Program &root,
return walker.result();
}
-// FIXME: FlangDriver
-// This option should be integrated with the real driver as the default of
-// RECURSIVE vs. NON_RECURSIVE may be changed by other command line options,
-// etc., etc.
-bool Fortran::lower::defaultRecursiveFunctionSetting() {
- return !nonRecursiveProcedures;
-}
-
void Fortran::lower::dumpPFT(llvm::raw_ostream &outputStream,
const lower::pft::Program &pft) {
PFTDumper{}.dumpPFT(outputStream, pft);
diff --git a/flang/test/Lower/OpenMP/atomic-read.f90 b/flang/test/Lower/OpenMP/atomic-read.f90
index 0f8a042be40a..7d479be1c10e 100644
--- a/flang/test/Lower/OpenMP/atomic-read.f90
+++ b/flang/test/Lower/OpenMP/atomic-read.f90
@@ -3,12 +3,12 @@
! This test checks the lowering of atomic read
!CHECK: func @_QQmain() {
-!CHECK: %[[VAR_A:.*]] = fir.address_of(@_QFEa) : !fir.ref<!fir.char<1>>
-!CHECK: %[[VAR_B:.*]] = fir.address_of(@_QFEb) : !fir.ref<!fir.char<1>>
+!CHECK: %[[VAR_A:.*]] = fir.alloca !fir.char<1> {bindc_name = "a", uniq_name = "_QFEa"}
+!CHECK: %[[VAR_B:.*]] = fir.alloca !fir.char<1> {bindc_name = "b", uniq_name = "_QFEb"}
!CHECK: %[[VAR_C:.*]] = fir.alloca !fir.logical<4> {bindc_name = "c", uniq_name = "_QFEc"}
!CHECK: %[[VAR_D:.*]] = fir.alloca !fir.logical<4> {bindc_name = "d", uniq_name = "_QFEd"}
-!CHECK: %[[VAR_E:.*]] = fir.address_of(@_QFEe) : !fir.ref<!fir.char<1,8>>
-!CHECK: %[[VAR_F:.*]] = fir.address_of(@_QFEf) : !fir.ref<!fir.char<1,8>>
+!CHECK: %[[VAR_E:.*]] = fir.alloca !fir.char<1,8> {bindc_name = "e", uniq_name = "_QFEe"}
+!CHECK: %[[VAR_F:.*]] = fir.alloca !fir.char<1,8> {bindc_name = "f", uniq_name = "_QFEf"}
!CHECK: %[[VAR_G:.*]] = fir.alloca f32 {bindc_name = "g", uniq_name = "_QFEg"}
!CHECK: %[[VAR_H:.*]] = fir.alloca f32 {bindc_name = "h", uniq_name = "_QFEh"}
!CHECK: %[[VAR_X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
diff --git a/flang/test/Lower/array-character.f90 b/flang/test/Lower/array-character.f90
index 82b77fbb6f66..eb99de02ecce 100644
--- a/flang/test/Lower/array-character.f90
+++ b/flang/test/Lower/array-character.f90
@@ -58,7 +58,7 @@ program p
! CHECK-DAG: %[[VAL_0:.*]] = arith.constant 4 : index
! CHECK-DAG: %[[VAL_1:.*]] = arith.constant 3 : index
! CHECK-DAG: %[[VAL_2:.*]] = arith.constant -1 : i32
- ! CHECK: %[[VAL_5:.*]] = fir.address_of(@_QFEc1) : !fir.ref<!fir.array<3x!fir.char<1,4>>>
+ ! CHECK: %[[VAL_5:.*]] = fir.alloca !fir.array<3x!fir.char<1,4>> {bindc_name = "c1", uniq_name = "_QFEc1"}
! CHECK: %[[VAL_6:.*]] = fir.address_of(@_QFEc2) : !fir.ref<!fir.array<3x!fir.char<1,4>>>
! CHECK: %[[VAL_7:.*]] = fir.address_of(@_QQcl.{{.*}}) : !fir.ref<!fir.char<1,
! CHECK: %[[VAL_8:.*]] = fir.convert %[[VAL_7]] : (!fir.ref<!fir.char<1,{{.*}}>>) -> !fir.ref<i8>
diff --git a/flang/test/Lower/array-expression-slice-1.f90 b/flang/test/Lower/array-expression-slice-1.f90
index b7eb6a73e698..bd10eb5749ce 100644
--- a/flang/test/Lower/array-expression-slice-1.f90
+++ b/flang/test/Lower/array-expression-slice-1.f90
@@ -18,10 +18,10 @@
! CHECK-DAG: %[[VAL_23:.*]] = arith.constant 1 : i32
! CHECK-DAG: %[[VAL_24:.*]] = arith.constant 0 : i32
! CHECK-DAG: %[[VAL_25:.*]] = fir.address_of(@_QFEa1) : !fir.ref<!fir.array<10x10xf32>>
-! CHECK-DAG: %[[VAL_26:.*]] = fir.address_of(@_QFEa2) : !fir.ref<!fir.array<3xf32>>
+! CHECK-DAG: %[[VAL_26:.*]] = fir.alloca !fir.array<3xf32> {bindc_name = "a2", uniq_name = "_QFEa2"}
! CHECK-DAG: %[[VAL_27:.*]] = fir.address_of(@_QFEa3) : !fir.ref<!fir.array<10xf32>>
! CHECK-DAG: %[[VAL_28:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFEi"}
-! CHECK-DAG: %[[VAL_29:.*]] = fir.address_of(@_QFEiv) : !fir.ref<!fir.array<3xi32>>
+! CHECK-DAG: %[[VAL_29:.*]] = fir.alloca !fir.array<3xi32> {bindc_name = "iv", uniq_name = "_QFEiv"}
! CHECK-DAG: %[[VAL_30:.*]] = fir.alloca i32 {bindc_name = "j", uniq_name = "_QFEj"}
! CHECK-DAG: %[[VAL_31:.*]] = fir.alloca i32 {bindc_name = "k", uniq_name = "_QFEk"}
! CHECK: fir.store %[[VAL_24]] to %[[VAL_31]] : !fir.ref<i32>
diff --git a/flang/test/Lower/associate-construct.f90 b/flang/test/Lower/associate-construct.f90
index 9ca14fd35aba..8aa5e50c611d 100644
--- a/flang/test/Lower/associate-construct.f90
+++ b/flang/test/Lower/associate-construct.f90
@@ -4,7 +4,7 @@
program p
! CHECK-DAG: [[I:%[0-9]+]] = fir.alloca i32 {{{.*}}uniq_name = "_QFEi"}
! CHECK-DAG: [[N:%[0-9]+]] = fir.alloca i32 {{{.*}}uniq_name = "_QFEn"}
- ! CHECK: [[T:%[0-9]+]] = fir.address_of(@_QFEt) : !fir.ref<!fir.array<3xi32>>
+ ! CHECK: [[T:%[0-9]+]] = fir.alloca !fir.array<3xi32> {bindc_name = "t", uniq_name = "_QFEt"}
integer :: n, foo, t(3)
! CHECK: [[N]]
! CHECK-COUNT-3: fir.coordinate_of [[T]]
diff --git a/flang/test/Lower/io-write.f90 b/flang/test/Lower/io-write.f90
index fa1424e710c5..5b9496feb197 100644
--- a/flang/test/Lower/io-write.f90
+++ b/flang/test/Lower/io-write.f90
@@ -6,8 +6,8 @@
character(3) string
write(string,getstring(6))
! CHECK: %[[Val_0:.*]] = fir.alloca i32 {adapt.valuebyref}
-! CHECK: %[[Val_1:.*]] = fir.address_of(@_QFEstring) : !fir.ref<!fir.char<1,3>>
! CHECK: %[[Const_3:.*]] = arith.constant 3 : index
+! CHECK: %[[Val_1:.*]] = fir.alloca !fir.char<1,3> {bindc_name = "string", uniq_name = "_QFEstring"}
! CHECK: %[[Val_2:.*]] = fir.convert %[[Val_1]] : (!fir.ref<!fir.char<1,3>>) -> !fir.ref<i8>
! CHECK: %[[Val_3:.*]] = fir.convert %[[Const_3]] : (index) -> i64
! CHECK: %[[Const_6:.*]] = arith.constant 6 : i32
diff --git a/flang/test/Lower/namelist.f90 b/flang/test/Lower/namelist.f90
index 576779086d15..14d1f2145660 100644
--- a/flang/test/Lower/namelist.f90
+++ b/flang/test/Lower/namelist.f90
@@ -2,7 +2,7 @@
! CHECK-LABEL: func @_QQmain
program p
- ! CHECK-DAG: [[ccc:%[0-9]+]] = fir.address_of(@_QFEccc) : !fir.ref<!fir.array<4x!fir.char<1,3>>>
+ ! CHECK-DAG: [[ccc:%[0-9]+]] = fir.alloca !fir.array<4x!fir.char<1,3>> {bindc_name = "ccc", uniq_name = "_QFEccc"}
! CHECK-DAG: [[jjj:%[0-9]+]] = fir.alloca i32 {bindc_name = "jjj", uniq_name = "_QFEjjj"}
character*3 ccc(4)
namelist /nnn/ jjj, ccc
@@ -17,7 +17,7 @@ program p
! CHECK: fir.insert_value
! CHECK: fir.address_of
! CHECK: fir.insert_value
- ! CHECK: fir.address_of(@_QFEccc.desc) : !fir.ref<!fir.box<!fir.ptr<!fir.array<4x!fir.char<1,3>>>>>
+ ! CHECK: fir.embox [[ccc]]
! CHECK: fir.insert_value
! CHECK: fir.alloca tuple<!fir.ref<i8>, i64, !fir.ref<!fir.array<2xtuple<!fir.ref<i8>, !fir.ref<!fir.box<none>>>>>>
! CHECK: fir.address_of
@@ -37,7 +37,7 @@ program p
! CHECK: fir.insert_value
! CHECK: fir.address_of
! CHECK: fir.insert_value
- ! CHECK: fir.address_of(@_QFEccc.desc) : !fir.ref<!fir.box<!fir.ptr<!fir.array<4x!fir.char<1,3>>>>>
+ ! CHECK: fir.embox [[ccc]]
! CHECK: fir.insert_value
! CHECK: fir.alloca tuple<!fir.ref<i8>, i64, !fir.ref<!fir.array<2xtuple<!fir.ref<i8>, !fir.ref<!fir.box<none>>>>>>
! CHECK: fir.address_of
@@ -86,4 +86,3 @@ subroutine global_pointer
! CHECK-DAG: fir.global linkonce @_QQcl.6A6A6A00 constant : !fir.char<1,4>
! CHECK-DAG: fir.global linkonce @_QQcl.63636300 constant : !fir.char<1,4>
! CHECK-DAG: fir.global linkonce @_QQcl.6E6E6E00 constant : !fir.char<1,4>
- ! CHECK-DAG: fir.global linkonce @_QFEccc.desc constant : !fir.box<!fir.ptr<!fir.array<4x!fir.char<1,3>>>>
More information about the flang-commits
mailing list