[flang] Remove double pointer indirection for _QQEnvironmentDefaults (#90615)

DavidTruby · web-flow · commit ecec1311fe05 · 2024-04-30T22:38:36.000+01:00
A double pointer was being passed to the call to FortranStart rather than just a pointer to the EnvironmentDefaults.list. This now passes `null` directly when there's no EnvironmentDefaults.list and passes the list directly when there is, removing the original global variable which was a pointer to a pointer containing null or the EnvironmentDefaults.list global. Fixes #90537
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h b/flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
@@ -27,6 +27,7 @@ class GlobalOp;
 
 namespace mlir {
 class Location;
+class Value;
 } // namespace mlir
 
 namespace Fortran::lower {
@@ -38,7 +39,7 @@ namespace fir::runtime {
 /// Create the list of environment variable defaults for the runtime to set. The
 /// form of the generated list is defined in the runtime header file
 /// environment-default-list.h
-fir::GlobalOp genEnvironmentDefaults(
+mlir::Value genEnvironmentDefaults(
     fir::FirOpBuilder &builder, mlir::Location loc,
     const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults);
 
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Main.h b/flang/include/flang/Optimizer/Builder/Runtime/Main.h
@@ -9,6 +9,9 @@
 #ifndef FORTRAN_OPTIMIZER_BUILDER_RUNTIME_MAIN_H
 #define FORTRAN_OPTIMIZER_BUILDER_RUNTIME_MAIN_H
 
+#include "flang/Lower/EnvironmentDefault.h"
+#include <vector>
+
 namespace mlir {
 class Location;
 } // namespace mlir
@@ -21,8 +24,7 @@ class GlobalOp;
 namespace fir::runtime {
 
 void genMain(fir::FirOpBuilder &builder, mlir::Location loc,
-             fir::GlobalOp &env);
-
+             const std::vector<Fortran::lower::EnvironmentDefault> &defs);
 }
 
 #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_MAIN_H
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
@@ -348,22 +348,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     createGlobalOutsideOfFunctionLowering(
         [&]() { typeInfoConverter.createTypeInfo(*this); });
 
-    // Create the list of any environment defaults for the runtime to set. The
-    // runtime default list is only created if there is a main program to ensure
-    // it only happens once and to provide consistent results if multiple files
-    // are compiled separately.
+    // Generate the `main` entry point if necessary
     if (hasMainProgram)
       createGlobalOutsideOfFunctionLowering([&]() {
-        // FIXME: Ideally, this would create a call to a runtime function
-        // accepting the list of environment defaults. That way, we would not
-        // need to add an extern pointer to the runtime and said pointer would
-        // not need to be generated even if no defaults are specified.
-        // However, generating main or changing when the runtime reads
-        // environment variables is required to do so.
-        auto env = fir::runtime::genEnvironmentDefaults(
-            *builder, toLocation(), bridge.getEnvironmentDefaults());
-
-        fir::runtime::genMain(*builder, toLocation(), env);
+        fir::runtime::genMain(*builder, toLocation(),
+                              bridge.getEnvironmentDefaults());
       });
 
     finalizeOpenACCLowering();
diff --git a/flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp b/flang/lib/Optimizer/Builder/Runtime/EnvironmentDefaults.cpp
@@ -13,7 +13,7 @@
 #include "flang/Optimizer/Support/InternalNames.h"
 #include "llvm/ADT/ArrayRef.h"
 
-fir::GlobalOp fir::runtime::genEnvironmentDefaults(
+mlir::Value fir::runtime::genEnvironmentDefaults(
     fir::FirOpBuilder &builder, mlir::Location loc,
     const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults) {
   std::string envDefaultListPtrName =
@@ -34,13 +34,8 @@ fir::GlobalOp fir::runtime::genEnvironmentDefaults(
 
   // If no defaults were specified, initialize with a null pointer.
   if (envDefaults.empty()) {
-    return builder.createGlobalConstant(
-        loc, envDefaultListRefTy, envDefaultListPtrName,
-        [&](fir::FirOpBuilder &builder) {
-          mlir::Value nullVal =
-              builder.createNullConstant(loc, envDefaultListRefTy);
-          builder.create<fir::HasValueOp>(loc, nullVal);
-        });
+    mlir::Value nullVal = builder.createNullConstant(loc, envDefaultListRefTy);
+    return nullVal;
   }
 
   // Create the Item list.
@@ -98,11 +93,7 @@ fir::GlobalOp fir::runtime::genEnvironmentDefaults(
       envDefaultListBuilder, linkOnce);
 
   // Define the pointer to the list used by the runtime.
-  return builder.createGlobalConstant(
-      loc, envDefaultListRefTy, envDefaultListPtrName,
-      [&](fir::FirOpBuilder &builder) {
-        mlir::Value addr = builder.create<fir::AddrOfOp>(
-            loc, envDefaultList.resultType(), envDefaultList.getSymbol());
-        builder.create<fir::HasValueOp>(loc, addr);
-      });
+  mlir::Value addr = builder.create<fir::AddrOfOp>(
+      loc, envDefaultList.resultType(), envDefaultList.getSymbol());
+  return addr;
 }
diff --git a/flang/lib/Optimizer/Builder/Runtime/Main.cpp b/flang/lib/Optimizer/Builder/Runtime/Main.cpp
@@ -7,8 +7,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "flang/Optimizer/Builder/Runtime/Main.h"
+#include "flang/Lower/EnvironmentDefault.h"
 #include "flang/Optimizer/Builder/BoxValue.h"
 #include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h"
 #include "flang/Optimizer/Builder/Runtime/RTBuilder.h"
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
@@ -18,8 +20,9 @@
 using namespace Fortran::runtime;
 
 /// Create a `int main(...)` that calls the Fortran entry point
-void fir::runtime::genMain(fir::FirOpBuilder &builder, mlir::Location loc,
-                           fir::GlobalOp &env) {
+void fir::runtime::genMain(
+    fir::FirOpBuilder &builder, mlir::Location loc,
+    const std::vector<Fortran::lower::EnvironmentDefault> &defs) {
   auto *context = builder.getContext();
   auto argcTy = builder.getDefaultIntegerType();
   auto ptrTy = mlir::LLVM::LLVMPointerType::get(context);
@@ -48,10 +51,14 @@ void fir::runtime::genMain(fir::FirOpBuilder &builder, mlir::Location loc,
   mlir::OpBuilder::InsertionGuard insertGuard(builder);
   builder.setInsertionPointToStart(block);
 
+  // Create the list of any environment defaults for the runtime to set. The
+  // runtime default list is only created if there is a main program to ensure
+  // it only happens once and to provide consistent results if multiple files
+  // are compiled separately.
+  auto env = fir::runtime::genEnvironmentDefaults(builder, loc, defs);
+
   llvm::SmallVector<mlir::Value, 4> args(block->getArguments());
-  auto envAddr =
-      builder.create<fir::AddrOfOp>(loc, env.getType(), env.getSymbol());
-  args.push_back(envAddr);
+  args.push_back(env);
 
   builder.create<fir::CallOp>(loc, startFn, args);
   builder.create<fir::CallOp>(loc, qqMainFn);
diff --git a/flang/test/Driver/emit-mlir.f90 b/flang/test/Driver/emit-mlir.f90
@@ -15,16 +15,12 @@
 ! CHECK-LABEL: func @_QQmain() {
 ! CHECK-NEXT:  return
 ! CHECK-NEXT: }
-! CHECK-NEXT: fir.global @_QQEnvironmentDefaults constant : !fir.ref<tuple<i[[int_size:.*]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>> {
-! CHECK-NEXT:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
-! CHECK-NEXT: fir.has_value  %[[VAL_0]] : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
-! CHECK-NEXT: }
 ! CHECK-NEXT: func.func private @_FortranAProgramStart(i32, !llvm.ptr, !llvm.ptr, !llvm.ptr)
 ! CHECK-NEXT: func.func private @_FortranAProgramEndStatement()
 ! CHECK-NEXT: func.func @main(%arg0: i32, %arg1: !llvm.ptr, %arg2: !llvm.ptr) -> i32 {
 ! CHECK-NEXT: %c0_i32 = arith.constant 0 : i32
-! CHECK-NEXT: %0 = fir.address_of(@_QQEnvironmentDefaults) : !fir.ref<tuple<i32, !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
-! CHECK-NEXT: ir.call @_FortranAProgramStart(%arg0, %arg1, %arg2, %0) {{.*}} : (i32, !llvm.ptr, !llvm.ptr, !fir.ref<tuple<i32, !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>) 
+! CHECK-NEXT: %0 = fir.zero_bits !fir.ref<tuple<i32, !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
+! CHECK-NEXT: fir.call @_FortranAProgramStart(%arg0, %arg1, %arg2, %0) {{.*}} : (i32, !llvm.ptr, !llvm.ptr, !fir.ref<tuple<i32, !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>) 
 ! CHECK-NEXT: fir.call @_QQmain() fastmath<contract> : () -> ()
 ! CHECK-NEXT: fir.call @_FortranAProgramEndStatement() {{.*}} : () -> ()
 ! CHECK-NEXT: return %c0_i32 : i32
diff --git a/flang/test/Lower/convert.f90 b/flang/test/Lower/convert.f90
@@ -11,6 +11,9 @@ program test
 ! Try to test that -fconvert=<value> flag results in a environment default list
 ! with the FORT_CONVERT option correctly specified.
 
+! ALL:  %0 = fir.address_of(@_QQEnvironmentDefaults.list) : !fir.ref<tuple<i32, !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
+! ALL: fir.call @_FortranAProgramStart(%arg0, %arg1, %arg2, %0)
+
 ! ALL: fir.global linkonce @_QQEnvironmentDefaults.items constant : !fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>> {
 ! ALL: %[[VAL_0:.*]] = fir.undefined !fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>
 ! ALL: %[[VAL_1:.*]] = fir.address_of(@[[FC_STR:.*]]) : !fir.ref<!fir.char<1,13>>
@@ -41,6 +44,3 @@ program test
 ! ALL: %[[VAL_4:.*]] = fir.insert_value %[[VAL_2]], %[[VAL_3]], [1 : index] : (tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>, !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>) -> tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>
 ! ALL: fir.has_value %[[VAL_4]] : tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>
 
-! ALL: fir.global @_QQEnvironmentDefaults constant : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>> {
-! ALL: %[[VAL_0:.*]] = fir.address_of(@_QQEnvironmentDefaults.list) : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
-! ALL: fir.has_value %[[VAL_0]] : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<1xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
diff --git a/flang/test/Lower/environment-defaults.f90 b/flang/test/Lower/environment-defaults.f90
@@ -5,8 +5,9 @@ program test
   continue
 end
 
-! Test that a null pointer is generated for environment defaults if nothing is specified
+! Test that a null pointer is passed for environment defaults if nothing is specified
 
-! CHECK: fir.global @_QQEnvironmentDefaults constant : !fir.ref<tuple<i[[int_size:.*]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>> {
-! CHECK:  %[[VAL_0:.*]] = fir.zero_bits !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
-! CHECK: fir.has_value  %[[VAL_0]] : !fir.ref<tuple<i[[int_size]], !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
+! CHECK-NOT: @_QQEnvironmentDefaults
+
+! CHECK: %0 = fir.zero_bits !fir.ref<tuple<i32, !fir.ref<!fir.array<0xtuple<!fir.ref<i8>, !fir.ref<i8>>>>>>
+! CHECK-NEXT: @_FortranAProgramStart(%arg0, %arg1, %arg2, %0)
