[flang] Remove double pointer indirection for _QQEnvironmentDefaults #90615
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
llvmbot
added
flang:driver
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-driver Author: David Truby (DavidTruby) ChangesA double pointer was being passed to the call to FortranStart rather than Fixes #90537 Full diff: https://github.com/llvm/llvm-project/pull/90615.diff 8 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h b/flang/include/flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h
index 216d3bcec137e1..2a0baaefd0cadf 100755
--- 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
index 62faf46e1fc778..5a98a80926c3d7 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Main.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Main.h
@@ -9,6 +9,7 @@
#ifndef FORTRAN_OPTIMIZER_BUILDER_RUNTIME_MAIN_H
#define FORTRAN_OPTIMIZER_BUILDER_RUNTIME_MAIN_H
+#include "flang/Lower/EnvironmentDefault.h"
namespace mlir {
class Location;
} // namespace mlir
@@ -21,7 +22,7 @@ class GlobalOp;
namespace fir::runtime {
void genMain(fir::FirOpBuilder &builder, mlir::Location loc,
- fir::GlobalOp &env);
+ const std::vector<Fortran::lower::EnvironmentDefault> &defs);
}
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index b42909eaaacc45..388d584e0e20d0 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -360,10 +360,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
// 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());
+ //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
index 6e280ac0c06cd2..722fd2db7ba3b1 100755
--- 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,9 @@ 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);
- });
+ return nullVal;
}
// Create the Item list.
@@ -98,11 +94,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);
- });
+ return addr;
}
diff --git a/flang/lib/Optimizer/Builder/Runtime/Main.cpp b/flang/lib/Optimizer/Builder/Runtime/Main.cpp
index 3b24fbca9cdb7c..408d2c707e993a 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Main.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Main.cpp
@@ -7,9 +7,11 @@
//===----------------------------------------------------------------------===//
#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/RTBuilder.h"
+#include "flang/Optimizer/Builder/Runtime/EnvironmentDefaults.h"
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/Dialect/FIRType.h"
#include "flang/Runtime/main.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,10 @@ void fir::runtime::genMain(fir::FirOpBuilder &builder, mlir::Location loc,
mlir::OpBuilder::InsertionGuard insertGuard(builder);
builder.setInsertionPointToStart(block);
+ 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
index 83bb8fc1eddcd2..2345a7cf53e40a 100644
--- 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
index b7c8b8dc20cc7b..75d0f844149ce8 100755
--- 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
index 700f7581bd6e7d..f5f41dabecc1d4 100755
--- 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)
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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 llvm#90537
tblah
reviewed
Apr 30, 2024
flang/lib/Lower/Bridge.cpp
Outdated
Comment on lines
363
to
364
| // auto env = fir::runtime::genEnvironmentDefaults( | ||
| // *builder, toLocation(), bridge.getEnvironmentDefaults()); |
I didn't need this for some reason but it seems necessary on some systems.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A double pointer was being passed to the call to FortranStart rather than
just a pointer to the EnvironmentDefaults.list. This now passes
nulldirectlywhen 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