-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Flang][Driver] Add a flag to control zero initialization of global v… #122144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-clang-driver Author: Kiran Chandramohan (kiranchandramohan) Changes…ariables Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize. Full diff: https://github.com/llvm/llvm-project/pull/122144.diff 7 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 52823430919de4..e8d18cf78e985a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3494,6 +3494,9 @@ def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;
def fno_strict_enums : Flag<["-"], "fno-strict-enums">, Group<f_Group>;
def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>,
Visibility<[ClangOption, FlangOption]>;
+def fno_zero_init_global_without_init : Flag<["-"], "fno-zero-init-global-without-init">, Group<f_Group>,
+ Visibility<[FlangOption, FC1Option]>,
+ HelpText<"Do not zero initialize globals without default initialization">;
def fno_pointer_tbaa : Flag<["-"], "fno-pointer-tbaa">, Group<f_Group>;
def fno_temp_file : Flag<["-"], "fno-temp-file">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>, HelpText<
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 75b10e88371ae7..609f5315426977 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -153,6 +153,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_fno_ppc_native_vec_elem_order,
+ options::OPT_fno_zero_init_global_without_init,
options::OPT_fppc_native_vec_elem_order});
}
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index 5a6debfdffe030..acce21f5faab8b 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -44,5 +44,8 @@ ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 0)
/// If false, assume that the shapes/types/allocation-status match.
ENUM_LOWERINGOPT(ReallocateLHS, unsigned, 1, 1)
+/// If true, initialize globals without initialization to zero.
+/// On by default.
+ENUM_LOWERINGOPT(ZeroInitGlobalsWithoutInit, unsigned, 1, 1)
#undef LOWERINGOPT
#undef ENUM_LOWERINGOPT
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 340efb1c63a5e5..fa91a0e2642bc8 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1373,6 +1373,12 @@ bool CompilerInvocation::createFromArgs(
invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
}
+ // -fno-zero-init-global-without-init
+ if (args.hasArg(
+ clang::driver::options::OPT_fno_zero_init_global_without_init)) {
+ invoc.loweringOpts.setZeroInitGlobalsWithoutInit(false);
+ }
+
// Preserve all the remark options requested, i.e. -Rpass, -Rpass-missed or
// -Rpass-analysis. This will be used later when processing and outputting the
// remarks generated by LLVM in ExecuteCompilerInvocation.cpp.
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 9ee42d5cd88002..7a422b07dd6c11 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -50,7 +50,6 @@ static llvm::cl::opt<bool>
allowAssumedRank("allow-assumed-rank",
llvm::cl::desc("Enable assumed rank lowering"),
llvm::cl::init(true));
-
#define DEBUG_TYPE "flang-lower-variable"
/// Helper to lower a scalar expression using a specific symbol mapping.
@@ -635,7 +634,11 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
global.setLinkName(builder.createCommonLinkage());
Fortran::lower::createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) {
- mlir::Value initValue = builder.create<fir::ZeroOp>(loc, symTy);
+ mlir::Value initValue;
+ if (converter.getLoweringOptions().getZeroInitGlobalsWithoutInit())
+ initValue = builder.create<fir::ZeroOp>(loc, symTy);
+ else
+ initValue = builder.create<fir::UndefOp>(loc, symTy);
builder.create<fir::HasValueOp>(loc, initValue);
});
}
diff --git a/flang/test/Lower/zero_init.f90 b/flang/test/Lower/zero_init.f90
new file mode 100644
index 00000000000000..76201eb0f0cbd0
--- /dev/null
+++ b/flang/test/Lower/zero_init.f90
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -fno-zero-init-global-without-init -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-NO-ZERO-INIT %s
+
+module m1
+ real :: x
+end module m1
+
+!CHECK-DEFAULT: fir.global @_QMm1Ex : f32 {
+!CHECK-DEFAULT: %[[UNDEF:.*]] = fir.zero_bits f32
+!CHECK-DEFAULT: fir.has_value %[[UNDEF]] : f32
+!CHECK-DEFAULT: }
+
+!CHECK-NO-ZERO-INIT: fir.global @_QMm1Ex : f32 {
+!CHECK-NO-ZERO-INIT: %[[UNDEF:.*]] = fir.undefined f32
+!CHECK-NO-ZERO-INIT: fir.has_value %[[UNDEF]] : f32
+!CHECK-NO-ZERO-INIT: }
diff --git a/flang/test/Lower/zero_init_default_init.f90 b/flang/test/Lower/zero_init_default_init.f90
new file mode 100644
index 00000000000000..41ca473451b910
--- /dev/null
+++ b/flang/test/Lower/zero_init_default_init.f90
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -fno-zero-init-global-without-init -emit-hlfir -o - %s | FileCheck %s
+
+module m2
+ type val
+ integer :: my_val = 1
+ end type val
+ type(val) :: v1
+end module m2
+
+!CHECK: fir.global @_QMm2Ev1 : !fir.type<_QMm2Tval{my_val:i32}> {
+!CHECK: %[[V1:.*]] = fir.undefined !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: %[[ONE:.*]] = arith.constant 1 : i32
+!CHECK: %[[V1_INIT:.*]] = fir.insert_value %[[V1]], %[[ONE]], ["my_val", !fir.type<_QMm2Tval{my_val:i32}>] : (!fir.type<_QMm2Tval{my_val:i32}>, i32) -> !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: fir.has_value %[[V1_INIT]] : !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: }
|
@llvm/pr-subscribers-clang Author: Kiran Chandramohan (kiranchandramohan) Changes…ariables Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize. Full diff: https://github.com/llvm/llvm-project/pull/122144.diff 7 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 52823430919de4..e8d18cf78e985a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3494,6 +3494,9 @@ def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;
def fno_strict_enums : Flag<["-"], "fno-strict-enums">, Group<f_Group>;
def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group<f_Group>,
Visibility<[ClangOption, FlangOption]>;
+def fno_zero_init_global_without_init : Flag<["-"], "fno-zero-init-global-without-init">, Group<f_Group>,
+ Visibility<[FlangOption, FC1Option]>,
+ HelpText<"Do not zero initialize globals without default initialization">;
def fno_pointer_tbaa : Flag<["-"], "fno-pointer-tbaa">, Group<f_Group>;
def fno_temp_file : Flag<["-"], "fno-temp-file">, Group<f_Group>,
Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>, HelpText<
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 75b10e88371ae7..609f5315426977 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -153,6 +153,7 @@ void Flang::addCodegenOptions(const ArgList &Args,
Args.addAllArgs(CmdArgs, {options::OPT_flang_experimental_hlfir,
options::OPT_flang_deprecated_no_hlfir,
options::OPT_fno_ppc_native_vec_elem_order,
+ options::OPT_fno_zero_init_global_without_init,
options::OPT_fppc_native_vec_elem_order});
}
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index 5a6debfdffe030..acce21f5faab8b 100644
--- a/flang/include/flang/Lower/LoweringOptions.def
+++ b/flang/include/flang/Lower/LoweringOptions.def
@@ -44,5 +44,8 @@ ENUM_LOWERINGOPT(IntegerWrapAround, unsigned, 1, 0)
/// If false, assume that the shapes/types/allocation-status match.
ENUM_LOWERINGOPT(ReallocateLHS, unsigned, 1, 1)
+/// If true, initialize globals without initialization to zero.
+/// On by default.
+ENUM_LOWERINGOPT(ZeroInitGlobalsWithoutInit, unsigned, 1, 1)
#undef LOWERINGOPT
#undef ENUM_LOWERINGOPT
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 340efb1c63a5e5..fa91a0e2642bc8 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1373,6 +1373,12 @@ bool CompilerInvocation::createFromArgs(
invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
}
+ // -fno-zero-init-global-without-init
+ if (args.hasArg(
+ clang::driver::options::OPT_fno_zero_init_global_without_init)) {
+ invoc.loweringOpts.setZeroInitGlobalsWithoutInit(false);
+ }
+
// Preserve all the remark options requested, i.e. -Rpass, -Rpass-missed or
// -Rpass-analysis. This will be used later when processing and outputting the
// remarks generated by LLVM in ExecuteCompilerInvocation.cpp.
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 9ee42d5cd88002..7a422b07dd6c11 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -50,7 +50,6 @@ static llvm::cl::opt<bool>
allowAssumedRank("allow-assumed-rank",
llvm::cl::desc("Enable assumed rank lowering"),
llvm::cl::init(true));
-
#define DEBUG_TYPE "flang-lower-variable"
/// Helper to lower a scalar expression using a specific symbol mapping.
@@ -635,7 +634,11 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
global.setLinkName(builder.createCommonLinkage());
Fortran::lower::createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) {
- mlir::Value initValue = builder.create<fir::ZeroOp>(loc, symTy);
+ mlir::Value initValue;
+ if (converter.getLoweringOptions().getZeroInitGlobalsWithoutInit())
+ initValue = builder.create<fir::ZeroOp>(loc, symTy);
+ else
+ initValue = builder.create<fir::UndefOp>(loc, symTy);
builder.create<fir::HasValueOp>(loc, initValue);
});
}
diff --git a/flang/test/Lower/zero_init.f90 b/flang/test/Lower/zero_init.f90
new file mode 100644
index 00000000000000..76201eb0f0cbd0
--- /dev/null
+++ b/flang/test/Lower/zero_init.f90
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -fno-zero-init-global-without-init -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-NO-ZERO-INIT %s
+
+module m1
+ real :: x
+end module m1
+
+!CHECK-DEFAULT: fir.global @_QMm1Ex : f32 {
+!CHECK-DEFAULT: %[[UNDEF:.*]] = fir.zero_bits f32
+!CHECK-DEFAULT: fir.has_value %[[UNDEF]] : f32
+!CHECK-DEFAULT: }
+
+!CHECK-NO-ZERO-INIT: fir.global @_QMm1Ex : f32 {
+!CHECK-NO-ZERO-INIT: %[[UNDEF:.*]] = fir.undefined f32
+!CHECK-NO-ZERO-INIT: fir.has_value %[[UNDEF]] : f32
+!CHECK-NO-ZERO-INIT: }
diff --git a/flang/test/Lower/zero_init_default_init.f90 b/flang/test/Lower/zero_init_default_init.f90
new file mode 100644
index 00000000000000..41ca473451b910
--- /dev/null
+++ b/flang/test/Lower/zero_init_default_init.f90
@@ -0,0 +1,16 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -fno-zero-init-global-without-init -emit-hlfir -o - %s | FileCheck %s
+
+module m2
+ type val
+ integer :: my_val = 1
+ end type val
+ type(val) :: v1
+end module m2
+
+!CHECK: fir.global @_QMm2Ev1 : !fir.type<_QMm2Tval{my_val:i32}> {
+!CHECK: %[[V1:.*]] = fir.undefined !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: %[[ONE:.*]] = arith.constant 1 : i32
+!CHECK: %[[V1_INIT:.*]] = fir.insert_value %[[V1]], %[[ONE]], ["my_val", !fir.type<_QMm2Tval{my_val:i32}>] : (!fir.type<_QMm2Tval{my_val:i32}>, i32) -> !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: fir.has_value %[[V1_INIT]] : !fir.type<_QMm2Tval{my_val:i32}>
+!CHECK: }
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks Kiran.
…ariables Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize.
c4ecdf6
to
4835015
Compare
Is the name of the flag alrite? |
Haha. You read my mind. I spent some time thinking about it and even consulted the Fortran standard. They refer to default initialization as well, so I eventually decided that it was ok. Maybe |
Perhaps leave this up for a bit longer to see if we can think of something better. Or others may have some suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Kiran!
Regarding the name, I never have strong opinions about them. Maybe fno-zero-init-globals
is enough (the description talks about the without-init part, and it is rather obvious I think that this will not affect/control user define initialization). I also like -fno-init-uninitialized-globals
.
gfortran has -finit-local-zero
which flang will likely eventually have. It has no such options for globals (it is putting everything is .bss ASFAIK). Maybe -fno-init-global-zero
would be consistent whit that existing name.
Maybe ask in the flang slack channel for opinions?
Thanks @tarunprabhu @jeanPerier, Changed to |
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beware there is a formatting issue (see github CI). LGTM otherwise, thanks Kiran!
Flang does not use the Marshalling fields and the setting of CodegenOpts from Options.td.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/14422 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/17277 Here is the relevant piece of the build log for the reference
|
…global v…" (#123067) Reverts #122144 Reverting due to CI failure https://lab.llvm.org/buildbot/#/builders/89/builds/14422
…ization of global v…" (#123067) Reverts llvm/llvm-project#122144 Reverting due to CI failure https://lab.llvm.org/buildbot/#/builders/89/builds/14422
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/20/builds/7750 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/29/builds/9053 Here is the relevant piece of the build log for the reference
|
…ariables
Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize.