Skip to content

Revert "Revert "[Flang][Driver] Add a flag to control zero initializa… #123097

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

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

kiranchandramohan
Copy link
Contributor

…tion of global v…" (#123067)"

This reverts commit 44ba43a.

Adds the flag to bbc as well.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category flang:fir-hlfir labels Jan 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-flang-fir-hlfir
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-flang-driver

Author: Kiran Chandramohan (kiranchandramohan)

Changes

…tion of global v…" (#123067)"

This reverts commit 44ba43a.

Adds the flag to bbc as well.


Full diff: https://github.com/llvm/llvm-project/pull/123097.diff

9 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+3-1)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+3)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+8)
  • (modified) flang/lib/Lower/ConvertVariable.cpp (+5-1)
  • (added) flang/test/Driver/fno-zero-init.f90 (+9)
  • (added) flang/test/Lower/zero_init.f90 (+20)
  • (added) flang/test/Lower/zero_init_default_init.f90 (+22)
  • (modified) flang/tools/bbc/bbc.cpp (+6)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2721c1b5d8dc55..dacfca910acc40 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3505,6 +3505,11 @@ 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]>;
+defm init_global_zero : BoolOptionWithoutMarshalling<"f", "init-global-zero",
+  PosFlag<SetTrue, [], [FlangOption, FC1Option],
+          "Zero initialize globals without default initialization (default)">,
+  NegFlag<SetFalse, [], [FlangOption, FC1Option],
+          "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 a7d0cc99f27d2d..c46e8222a9631a 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -155,7 +155,9 @@ void Flang::addCodegenOptions(const ArgList &Args,
                    options::OPT_flang_deprecated_no_hlfir,
                    options::OPT_fno_ppc_native_vec_elem_order,
                    options::OPT_fppc_native_vec_elem_order,
-                   options::OPT_ftime_report, options::OPT_ftime_report_EQ});
+                   options::OPT_finit_global_zero,
+                   options::OPT_fno_init_global_zero, options::OPT_ftime_report,
+                   options::OPT_ftime_report_EQ});
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index 5a6debfdffe030..396c91948be36b 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(InitGlobalZero, unsigned, 1, 1)
 #undef LOWERINGOPT
 #undef ENUM_LOWERINGOPT
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 5e7127313c1335..78d1199c19749b 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1373,6 +1373,14 @@ bool CompilerInvocation::createFromArgs(
     invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
   }
 
+  // -f[no-]init-global-zero
+  if (args.hasFlag(clang::driver::options::OPT_finit_global_zero,
+                   clang::driver::options::OPT_fno_init_global_zero,
+                   /*default=*/true))
+    invoc.loweringOpts.setInitGlobalZero(true);
+  else
+    invoc.loweringOpts.setInitGlobalZero(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..87236dc293ebbc 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -635,7 +635,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().getInitGlobalZero())
+            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/Driver/fno-zero-init.f90 b/flang/test/Driver/fno-zero-init.f90
new file mode 100644
index 00000000000000..2ffa10dd040d52
--- /dev/null
+++ b/flang/test/Driver/fno-zero-init.f90
@@ -0,0 +1,9 @@
+! Check that the driver passes through -f[no-]init-global-zero:
+! RUN: %flang -### -S -finit-global-zero %s -o - 2>&1 | FileCheck --check-prefix=CHECK-POS %s
+! RUN: %flang -### -S -fno-init-global-zero %s -o - 2>&1 | FileCheck --check-prefix=CHECK-NEG %s
+! Check that the compiler accepts -f[no-]init-global-zero:
+! RUN: %flang_fc1 -emit-hlfir -finit-global-zero %s -o -
+! RUN: %flang_fc1 -emit-hlfir -fno-init-global-zero %s -o -
+
+! CHECK-POS: "-fc1"{{.*}}"-finit-global-zero"
+! CHECK-NEG: "-fc1"{{.*}}"-fno-init-global-zero"
diff --git a/flang/test/Lower/zero_init.f90 b/flang/test/Lower/zero_init.f90
new file mode 100644
index 00000000000000..5ed6f2247de3b2
--- /dev/null
+++ b/flang/test/Lower/zero_init.f90
@@ -0,0 +1,20 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -finit-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -fno-init-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-NO-ZERO-INIT %s
+! RUN: bbc -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: bbc -finit-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: bbc -finit-global-zero=false -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..e2d1f545e35a57
--- /dev/null
+++ b/flang/test/Lower/zero_init_default_init.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -finit-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -fno-init-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -finit-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -finit-global-zero=false -emit-hlfir -o - %s | FileCheck %s
+
+! Test that the flag does not affect globals with default init
+
+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:  }
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 7efc460be86795..dafbcd856389a8 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -234,6 +234,11 @@ static llvm::cl::opt<bool> integerWrapAround(
     llvm::cl::desc("Treat signed integer overflow as two's complement"),
     llvm::cl::init(false));
 
+static llvm::cl::opt<bool> initGlobalZero(
+    "finit-global-zero",
+    llvm::cl::desc("Zero initialize globals without default initialization"),
+    llvm::cl::init(true));
+
 static llvm::cl::opt<bool>
     reallocateLHS("frealloc-lhs",
                   llvm::cl::desc("Follow Fortran 2003 rules for (re)allocating "
@@ -381,6 +386,7 @@ static llvm::LogicalResult convertFortranSourceToMLIR(
   loweringOptions.setNoPPCNativeVecElemOrder(enableNoPPCNativeVecElemOrder);
   loweringOptions.setLowerToHighLevelFIR(useHLFIR || emitHLFIR);
   loweringOptions.setIntegerWrapAround(integerWrapAround);
+  loweringOptions.setInitGlobalZero(initGlobalZero);
   loweringOptions.setReallocateLHS(reallocateLHS);
   std::vector<Fortran::lower::EnvironmentDefault> envDefaults = {};
   Fortran::frontend::TargetOptions targetOpts;

@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-clang

Author: Kiran Chandramohan (kiranchandramohan)

Changes

…tion of global v…" (#123067)"

This reverts commit 44ba43a.

Adds the flag to bbc as well.


Full diff: https://github.com/llvm/llvm-project/pull/123097.diff

9 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+3-1)
  • (modified) flang/include/flang/Lower/LoweringOptions.def (+3)
  • (modified) flang/lib/Frontend/CompilerInvocation.cpp (+8)
  • (modified) flang/lib/Lower/ConvertVariable.cpp (+5-1)
  • (added) flang/test/Driver/fno-zero-init.f90 (+9)
  • (added) flang/test/Lower/zero_init.f90 (+20)
  • (added) flang/test/Lower/zero_init_default_init.f90 (+22)
  • (modified) flang/tools/bbc/bbc.cpp (+6)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2721c1b5d8dc55..dacfca910acc40 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3505,6 +3505,11 @@ 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]>;
+defm init_global_zero : BoolOptionWithoutMarshalling<"f", "init-global-zero",
+  PosFlag<SetTrue, [], [FlangOption, FC1Option],
+          "Zero initialize globals without default initialization (default)">,
+  NegFlag<SetFalse, [], [FlangOption, FC1Option],
+          "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 a7d0cc99f27d2d..c46e8222a9631a 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -155,7 +155,9 @@ void Flang::addCodegenOptions(const ArgList &Args,
                    options::OPT_flang_deprecated_no_hlfir,
                    options::OPT_fno_ppc_native_vec_elem_order,
                    options::OPT_fppc_native_vec_elem_order,
-                   options::OPT_ftime_report, options::OPT_ftime_report_EQ});
+                   options::OPT_finit_global_zero,
+                   options::OPT_fno_init_global_zero, options::OPT_ftime_report,
+                   options::OPT_ftime_report_EQ});
 }
 
 void Flang::addPicOptions(const ArgList &Args, ArgStringList &CmdArgs) const {
diff --git a/flang/include/flang/Lower/LoweringOptions.def b/flang/include/flang/Lower/LoweringOptions.def
index 5a6debfdffe030..396c91948be36b 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(InitGlobalZero, unsigned, 1, 1)
 #undef LOWERINGOPT
 #undef ENUM_LOWERINGOPT
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 5e7127313c1335..78d1199c19749b 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1373,6 +1373,14 @@ bool CompilerInvocation::createFromArgs(
     invoc.loweringOpts.setNoPPCNativeVecElemOrder(true);
   }
 
+  // -f[no-]init-global-zero
+  if (args.hasFlag(clang::driver::options::OPT_finit_global_zero,
+                   clang::driver::options::OPT_fno_init_global_zero,
+                   /*default=*/true))
+    invoc.loweringOpts.setInitGlobalZero(true);
+  else
+    invoc.loweringOpts.setInitGlobalZero(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..87236dc293ebbc 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -635,7 +635,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().getInitGlobalZero())
+            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/Driver/fno-zero-init.f90 b/flang/test/Driver/fno-zero-init.f90
new file mode 100644
index 00000000000000..2ffa10dd040d52
--- /dev/null
+++ b/flang/test/Driver/fno-zero-init.f90
@@ -0,0 +1,9 @@
+! Check that the driver passes through -f[no-]init-global-zero:
+! RUN: %flang -### -S -finit-global-zero %s -o - 2>&1 | FileCheck --check-prefix=CHECK-POS %s
+! RUN: %flang -### -S -fno-init-global-zero %s -o - 2>&1 | FileCheck --check-prefix=CHECK-NEG %s
+! Check that the compiler accepts -f[no-]init-global-zero:
+! RUN: %flang_fc1 -emit-hlfir -finit-global-zero %s -o -
+! RUN: %flang_fc1 -emit-hlfir -fno-init-global-zero %s -o -
+
+! CHECK-POS: "-fc1"{{.*}}"-finit-global-zero"
+! CHECK-NEG: "-fc1"{{.*}}"-fno-init-global-zero"
diff --git a/flang/test/Lower/zero_init.f90 b/flang/test/Lower/zero_init.f90
new file mode 100644
index 00000000000000..5ed6f2247de3b2
--- /dev/null
+++ b/flang/test/Lower/zero_init.f90
@@ -0,0 +1,20 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -finit-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: %flang_fc1 -fno-init-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-NO-ZERO-INIT %s
+! RUN: bbc -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: bbc -finit-global-zero -emit-hlfir -o - %s | FileCheck --check-prefix=CHECK-DEFAULT %s
+! RUN: bbc -finit-global-zero=false -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..e2d1f545e35a57
--- /dev/null
+++ b/flang/test/Lower/zero_init_default_init.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -finit-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: %flang_fc1 -fno-init-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -finit-global-zero -emit-hlfir -o - %s | FileCheck %s
+! RUN: bbc -finit-global-zero=false -emit-hlfir -o - %s | FileCheck %s
+
+! Test that the flag does not affect globals with default init
+
+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:  }
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 7efc460be86795..dafbcd856389a8 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -234,6 +234,11 @@ static llvm::cl::opt<bool> integerWrapAround(
     llvm::cl::desc("Treat signed integer overflow as two's complement"),
     llvm::cl::init(false));
 
+static llvm::cl::opt<bool> initGlobalZero(
+    "finit-global-zero",
+    llvm::cl::desc("Zero initialize globals without default initialization"),
+    llvm::cl::init(true));
+
 static llvm::cl::opt<bool>
     reallocateLHS("frealloc-lhs",
                   llvm::cl::desc("Follow Fortran 2003 rules for (re)allocating "
@@ -381,6 +386,7 @@ static llvm::LogicalResult convertFortranSourceToMLIR(
   loweringOptions.setNoPPCNativeVecElemOrder(enableNoPPCNativeVecElemOrder);
   loweringOptions.setLowerToHighLevelFIR(useHLFIR || emitHLFIR);
   loweringOptions.setIntegerWrapAround(integerWrapAround);
+  loweringOptions.setInitGlobalZero(initGlobalZero);
   loweringOptions.setReallocateLHS(reallocateLHS);
   std::vector<Fortran::lower::EnvironmentDefault> envDefaults = {};
   Fortran::frontend::TargetOptions targetOpts;

@kiranchandramohan
Copy link
Contributor Author

kiranchandramohan commented Jan 15, 2025

Apologies for the churn here. But the previous patch (#122144) was causing some non-deterministic failures in flang/test/Lower/module_use.f90. I am assuming this is because the LoweringOpt (ENUM_LOWERINGOPT(InitGlobalZero, unsigned, 1, 1)
) is not set to a reasonable value in the bbc flow. (there is a default, not clear why it is not being set in the bbc flow). I am fixing this by adding a similar flag to bbc.

Another reason I thought was the following.

The test that fails is flang/test/Lower/module_use.f90. This test also compiles another test flang/test/Lower/module_definition.f90. Is there an issue with compiling the same test with modules two times and can they interact badly? This used to be the case in Classic Flang.

@tarunprabhu
Copy link
Contributor

The test that fails is flang/test/Lower/module_use.f90. This test also compiles another test flang/test/Lower/module_definition.f90. Is there an issue with compiling the same test with modules two times and can they interact badly? This used to be the case in Classic Flang.

If the tests happen to run in parallel, they could clobber the .mod file if both write the module file to the same directory. This is the case with gfortran, but I haven't explicitly tried such things with flang.

Specifying an explicit -J with a temporary directory should resolve that. I know that %t in lit gives a temporary file. I don't know if you can get a temporary directory in a similar fashion.

@jeanPerier
Copy link
Contributor

+1 with @tarunprabhu testing suggestion. I am not opposed adding the flag to bbc, but it should really not be the root cause of the issue.

In the failing log, you can see that semantics mistook y(1) for a call ( the FIR output contains a fir.call @_QPy(%0)). This indeed indicates that something likely went wrong with the module file and that y was not read from it as expected. So it doe not look like a random setting of the InitGlobalZero issue, but a module file issue. I have no idea why your patch exposed it. I am guessing it may already have randomly failed and we do not know it because no flang committers had done a patch at the same time, so nobody probably investigated it.

@kiranchandramohan
Copy link
Contributor Author

Thanks @jeanPerier @tarunprabhu. I have created #123215 to use a temporary directory for the test.

I will keep the bbc option in this patch as well.

…tion of global v…" (llvm#123067)"

This reverts commit 44ba43a.

Adds the flag to bbc as well.
Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Kiran, LGTM

@kiranchandramohan kiranchandramohan merged commit 8c63648 into llvm:main Jan 17, 2025
8 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 17, 2025

LLVM Buildbot has detected a new failure on builder flang-aarch64-libcxx running on linaro-flang-aarch64-libcxx while building clang,flang at step 6 "test-build-unified-tree-check-flang".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/14577

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-flang) failure: test (failure)
******************** TEST 'Flang :: Lower/module_use.f90' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: rm -fr /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp && mkdir -p /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp
+ rm -fr /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp
+ mkdir -p /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp
RUN: at line 2: bbc -emit-fir -module /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_definition.f90
+ bbc -emit-fir -module /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_definition.f90
RUN: at line 3: bbc -emit-fir -J /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90 -o - | /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90
+ bbc -emit-fir -J /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/tools/flang/test/Lower/Output/module_use.f90.tmp /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90 -o -
+ /home/tcwg-buildbot/worker/flang-aarch64-libcxx/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90
/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90:17:15: error: CHECK-DAG: expected string not found in input
 ! CHECK-DAG: fir.address_of(@_QMm1Ey) : !fir.ref<!fir.array<100xi32>>
              ^
<stdin>:5:23: note: scanning from here
 func.func @_QPm1use() -> f32 {
                      ^
<stdin>:10:7: note: possible intended match here
 %3 = fir.address_of(@_QMm1Ex) : !fir.ref<f32>
      ^
/home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90:44:14: error: CHECK-DAG: expected string not found in input
! CHECK-DAG: fir.global @_QMm1Ey : !fir.array<100xi32>
             ^
<stdin>:20:31: note: scanning from here
 func.func @_QPmodcommon1use() -> f32 {
                              ^
<stdin>:49:2: note: possible intended match here
 fir.global @_QMm1Ex : f32
 ^

Input file: <stdin>
Check file: /home/tcwg-buildbot/worker/flang-aarch64-libcxx/llvm-project/flang/test/Lower/module_use.f90

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          1: module attributes {dlti.dl_spec = #dlti.dl_spec<i32 = dense<32> : vector<2xi64>, !llvm.ptr<270> = dense<32> : vector<4xi64>, f64 = dense<64> : vector<2xi64>, f16 = dense<16> : vector<2xi64>, !llvm.ptr<272> = dense<64> : vector<4xi64>, !llvm.ptr<271> = dense<32> : vector<4xi64>, f128 = dense<128> : vector<2xi64>, i8 = dense<[8, 32]> : vector<2xi64>, i16 = dense<[16, 32]> : vector<2xi64>, i64 = dense<64> : vector<2xi64>, i128 = dense<128> : vector<2xi64>, !llvm.ptr = dense<64> : vector<4xi64>, i1 = dense<8> : vector<2xi64>, "dlti.endianness" = "little", "dlti.stack_alignment" = 128 : i64>, fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32", llvm.ident = "flang version 20.0.0 (https://github.com/llvm/llvm-project.git 8c63648117f1e1705943903b149f36ab8a4df1e5)", llvm.target_triple = "aarch64-unknown-linux-gnu"} { 
          2:  fir.global common @__BLNK__(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8> 
          3:  fir.global common @named1_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8> 
          4:  fir.global common @named2_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8> 
          5:  func.func @_QPm1use() -> f32 { 
dag:17'0                           X~~~~~~~~~ error: no match found
          6:  %c1_i32 = arith.constant 1 : i32 
dag:17'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          7:  %0 = fir.alloca i32 {adapt.valuebyref} 
dag:17'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          8:  %1 = fir.alloca f32 {bindc_name = "m1use", uniq_name = "_QFm1useEm1use"} 
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants