[Flang][Driver] Add a flag to control zero initialization of global v… #122144
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kiranchandramohan
requested review from
DavidTruby,
jeanPerier and
tarunprabhu
llvmbot
added
clang
|
@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: }
|
…ariables Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize.
kiranchandramohan
force-pushed
the
zero-init
branch
from
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 |
jeanPerier
approved these changes
Jan 9, 2025
There was a problem hiding this comment.
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?
carlocab
reviewed
Jan 9, 2025
Thanks @tarunprabhu @jeanPerier, Changed to |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jeanPerier
approved these changes
Jan 14, 2025
carlocab
reviewed
Jan 14, 2025
carlocab
approved these changes
Jan 14, 2025
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 |
kiranchandramohan
added a commit
that referenced
this pull request
Jan 15, 2025
…global v…" (#123067) Reverts #122144 Reverting due to CI failure https://lab.llvm.org/buildbot/#/builders/89/builds/14422
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 15, 2025
…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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ariables
Patch adds a flag to control zero initialization of global variables without default initialization. The default is to zero initialize.