-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[flang] Emit fir.global
in the global address space
#146653
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
Instead of emitting globals in the program/default address space, emit them in the global address space. This also requires changes how address of code-gen is handled, we need to cast to the default address space to prevent code-gen issues.
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-backend-amdgpu Author: Kareem Ergawy (ergawy) ChangesInstead of emitting globals in the program/default address space, emit them in the global address space. This also requires changes how address of code-gen is handled, we need to cast to the default address space to prevent code-gen issues. Full diff: https://github.com/llvm/llvm-project/pull/146653.diff 6 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index e1eaab3346901..f2c363e09e9e6 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -900,6 +900,10 @@ uint64_t getAllocaAddressSpace(const mlir::DataLayout *dataLayout);
llvm::SmallVector<mlir::Value> deduceOptimalExtents(mlir::ValueRange extents1,
mlir::ValueRange extents2);
+uint64_t getGlobalAddressSpace(mlir::DataLayout *dataLayout);
+
+uint64_t getProgramAddressSpace(mlir::DataLayout *dataLayout);
+
/// Given array extents generate code that sets them all to zeroes,
/// if the array is empty, e.g.:
/// %false = arith.constant false
diff --git a/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h b/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
index 7b1c14e4dfdc9..cbdcf6b501b8e 100644
--- a/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
+++ b/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
@@ -189,6 +189,9 @@ class ConvertFIRToLLVMPattern : public mlir::ConvertToLLVMPattern {
unsigned
getProgramAddressSpace(mlir::ConversionPatternRewriter &rewriter) const;
+ unsigned
+ getGlobalAddressSpace(mlir::ConversionPatternRewriter &rewriter) const;
+
const fir::FIRToLLVMPassOptions &options;
using ConvertToLLVMPattern::matchAndRewrite;
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 6ac87067f6511..b5cabdb830e5c 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -1867,6 +1867,20 @@ fir::factory::deduceOptimalExtents(mlir::ValueRange extents1,
return extents;
}
+uint64_t fir::factory::getGlobalAddressSpace(mlir::DataLayout *dataLayout) {
+ if (dataLayout)
+ if (mlir::Attribute addrSpace = dataLayout->getGlobalMemorySpace())
+ return mlir::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return 0;
+}
+
+uint64_t fir::factory::getProgramAddressSpace(mlir::DataLayout *dataLayout) {
+ if (dataLayout)
+ if (mlir::Attribute addrSpace = dataLayout->getProgramMemorySpace())
+ return mlir::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return 0;
+}
+
llvm::SmallVector<mlir::Value> fir::factory::updateRuntimeExtentsForEmptyArrays(
fir::FirOpBuilder &builder, mlir::Location loc, mlir::ValueRange extents) {
if (extents.size() <= 1)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 2b018912b40e4..09038f6e4d3ae 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -137,6 +137,38 @@ addLLVMOpBundleAttrs(mlir::ConversionPatternRewriter &rewriter,
}
namespace {
+
+mlir::Value replaceWithAddrOfOrASCast(mlir::ConversionPatternRewriter &rewriter,
+ mlir::Location loc,
+ std::uint64_t globalAS,
+ std::uint64_t programAS,
+ llvm::StringRef symName, mlir::Type type,
+ mlir::Operation *replaceOp = nullptr) {
+ if (mlir::isa<mlir::LLVM::LLVMPointerType>(type)) {
+ if (globalAS != programAS) {
+ auto llvmAddrOp = rewriter.create<mlir::LLVM::AddressOfOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddrSpaceCastOp>(
+ replaceOp, ::getLlvmPtrType(rewriter.getContext(), programAS),
+ llvmAddrOp);
+ return rewriter.create<mlir::LLVM::AddrSpaceCastOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), programAS), llvmAddrOp);
+ }
+
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
+ replaceOp, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ return rewriter.create<mlir::LLVM::AddressOfOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ }
+
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(replaceOp, type,
+ symName);
+ return rewriter.create<mlir::LLVM::AddressOfOp>(loc, type, symName);
+}
+
/// Lower `fir.address_of` operation to `llvm.address_of` operation.
struct AddrOfOpConversion : public fir::FIROpConversion<fir::AddrOfOp> {
using FIROpConversion::FIROpConversion;
@@ -144,9 +176,15 @@ struct AddrOfOpConversion : public fir::FIROpConversion<fir::AddrOfOp> {
llvm::LogicalResult
matchAndRewrite(fir::AddrOfOp addr, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
- auto ty = convertType(addr.getType());
- rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
- addr, ty, addr.getSymbol().getRootReference().getValue());
+ auto global = addr->getParentOfType<mlir::ModuleOp>()
+ .lookupSymbol<mlir::LLVM::GlobalOp>(addr.getSymbol());
+ replaceWithAddrOfOrASCast(
+ rewriter, addr->getLoc(),
+ global ? global.getAddrSpace() : getGlobalAddressSpace(rewriter),
+ getProgramAddressSpace(rewriter),
+ global ? global.getSymName()
+ : addr.getSymbol().getRootReference().getValue(),
+ convertType(addr.getType()), addr);
return mlir::success();
}
};
@@ -1306,13 +1344,18 @@ getTypeDescriptor(ModOpTy mod, mlir::ConversionPatternRewriter &rewriter,
? fir::NameUniquer::getTypeDescriptorAssemblyName(recType.getName())
: fir::NameUniquer::getTypeDescriptorName(recType.getName());
mlir::Type llvmPtrTy = ::getLlvmPtrType(mod.getContext());
+ mlir::DataLayout dataLayout(mod);
if (auto global = mod.template lookupSymbol<fir::GlobalOp>(name))
- return rewriter.create<mlir::LLVM::AddressOfOp>(loc, llvmPtrTy,
- global.getSymName());
+ return replaceWithAddrOfOrASCast(
+ rewriter, loc, fir::factory::getGlobalAddressSpace(&dataLayout),
+ fir::factory::getProgramAddressSpace(&dataLayout), global.getSymName(),
+ llvmPtrTy);
// The global may have already been translated to LLVM.
if (auto global = mod.template lookupSymbol<mlir::LLVM::GlobalOp>(name))
- return rewriter.create<mlir::LLVM::AddressOfOp>(loc, llvmPtrTy,
- global.getSymName());
+ return replaceWithAddrOfOrASCast(
+ rewriter, loc, global.getAddrSpace(),
+ fir::factory::getProgramAddressSpace(&dataLayout), global.getSymName(),
+ llvmPtrTy);
// Type info derived types do not have type descriptors since they are the
// types defining type descriptors.
if (options.ignoreMissingTypeDescriptors ||
@@ -3130,8 +3173,8 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
mlir::SymbolRefAttr comdat;
llvm::ArrayRef<mlir::NamedAttribute> attrs;
auto g = rewriter.create<mlir::LLVM::GlobalOp>(
- loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0, 0,
- false, false, comdat, attrs, dbgExprs);
+ loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0,
+ getGlobalAddressSpace(rewriter), false, false, comdat, attrs, dbgExprs);
if (global.getAlignment() && *global.getAlignment() > 0)
g.setAlignment(*global.getAlignment());
diff --git a/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp b/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
index 12021deb4bd97..5c9c9c34caac1 100644
--- a/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
+++ b/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
@@ -365,4 +365,19 @@ unsigned ConvertFIRToLLVMPattern::getProgramAddressSpace(
return defaultAddressSpace;
}
+unsigned ConvertFIRToLLVMPattern::getGlobalAddressSpace(
+ mlir::ConversionPatternRewriter &rewriter) const {
+ mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
+ assert(parentOp != nullptr &&
+ "expected insertion block to have parent operation");
+ auto module = mlir::isa<mlir::ModuleOp>(parentOp)
+ ? mlir::cast<mlir::ModuleOp>(parentOp)
+ : parentOp->getParentOfType<mlir::ModuleOp>();
+ if (module)
+ if (mlir::Attribute addrSpace =
+ mlir::DataLayout(module).getGlobalMemorySpace())
+ return llvm::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return defaultAddressSpace;
+}
+
} // namespace fir
diff --git a/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90 b/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90
new file mode 100644
index 0000000000000..249d403057b6c
--- /dev/null
+++ b/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90
@@ -0,0 +1,18 @@
+!REQUIRES: amdgpu-registered-target
+
+!RUN: %flang_fc1 -emit-llvm -triple amdgcn-amd-amdhsa -target-cpu gfx908 %s -o - | FileCheck %s
+
+subroutine maintest
+ implicit none
+
+ type r1_t
+ end type r1_t
+
+ type(r1_t), pointer :: A
+end subroutine
+
+! CHECK: @[[TYPE_DESC:.*XdtXr1_t]] = linkonce_odr addrspace(1) constant %_QM__fortran_type_infoTderivedtype
+
+! CHECK: define void @maintest_() {{.*}} {
+! CHECK: store { {{.*}} } { {{.*}}, ptr addrspacecast (ptr addrspace(1) @[[TYPE_DESC]] to ptr), {{.*}} }, {{.*}}
+! CHECK: }
|
@llvm/pr-subscribers-flang-codegen Author: Kareem Ergawy (ergawy) ChangesInstead of emitting globals in the program/default address space, emit them in the global address space. This also requires changes how address of code-gen is handled, we need to cast to the default address space to prevent code-gen issues. Full diff: https://github.com/llvm/llvm-project/pull/146653.diff 6 Files Affected:
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index e1eaab3346901..f2c363e09e9e6 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -900,6 +900,10 @@ uint64_t getAllocaAddressSpace(const mlir::DataLayout *dataLayout);
llvm::SmallVector<mlir::Value> deduceOptimalExtents(mlir::ValueRange extents1,
mlir::ValueRange extents2);
+uint64_t getGlobalAddressSpace(mlir::DataLayout *dataLayout);
+
+uint64_t getProgramAddressSpace(mlir::DataLayout *dataLayout);
+
/// Given array extents generate code that sets them all to zeroes,
/// if the array is empty, e.g.:
/// %false = arith.constant false
diff --git a/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h b/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
index 7b1c14e4dfdc9..cbdcf6b501b8e 100644
--- a/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
+++ b/flang/include/flang/Optimizer/CodeGen/FIROpPatterns.h
@@ -189,6 +189,9 @@ class ConvertFIRToLLVMPattern : public mlir::ConvertToLLVMPattern {
unsigned
getProgramAddressSpace(mlir::ConversionPatternRewriter &rewriter) const;
+ unsigned
+ getGlobalAddressSpace(mlir::ConversionPatternRewriter &rewriter) const;
+
const fir::FIRToLLVMPassOptions &options;
using ConvertToLLVMPattern::matchAndRewrite;
diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
index 6ac87067f6511..b5cabdb830e5c 100644
--- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp
+++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp
@@ -1867,6 +1867,20 @@ fir::factory::deduceOptimalExtents(mlir::ValueRange extents1,
return extents;
}
+uint64_t fir::factory::getGlobalAddressSpace(mlir::DataLayout *dataLayout) {
+ if (dataLayout)
+ if (mlir::Attribute addrSpace = dataLayout->getGlobalMemorySpace())
+ return mlir::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return 0;
+}
+
+uint64_t fir::factory::getProgramAddressSpace(mlir::DataLayout *dataLayout) {
+ if (dataLayout)
+ if (mlir::Attribute addrSpace = dataLayout->getProgramMemorySpace())
+ return mlir::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return 0;
+}
+
llvm::SmallVector<mlir::Value> fir::factory::updateRuntimeExtentsForEmptyArrays(
fir::FirOpBuilder &builder, mlir::Location loc, mlir::ValueRange extents) {
if (extents.size() <= 1)
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 2b018912b40e4..09038f6e4d3ae 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -137,6 +137,38 @@ addLLVMOpBundleAttrs(mlir::ConversionPatternRewriter &rewriter,
}
namespace {
+
+mlir::Value replaceWithAddrOfOrASCast(mlir::ConversionPatternRewriter &rewriter,
+ mlir::Location loc,
+ std::uint64_t globalAS,
+ std::uint64_t programAS,
+ llvm::StringRef symName, mlir::Type type,
+ mlir::Operation *replaceOp = nullptr) {
+ if (mlir::isa<mlir::LLVM::LLVMPointerType>(type)) {
+ if (globalAS != programAS) {
+ auto llvmAddrOp = rewriter.create<mlir::LLVM::AddressOfOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddrSpaceCastOp>(
+ replaceOp, ::getLlvmPtrType(rewriter.getContext(), programAS),
+ llvmAddrOp);
+ return rewriter.create<mlir::LLVM::AddrSpaceCastOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), programAS), llvmAddrOp);
+ }
+
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
+ replaceOp, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ return rewriter.create<mlir::LLVM::AddressOfOp>(
+ loc, getLlvmPtrType(rewriter.getContext(), globalAS), symName);
+ }
+
+ if (replaceOp)
+ return rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(replaceOp, type,
+ symName);
+ return rewriter.create<mlir::LLVM::AddressOfOp>(loc, type, symName);
+}
+
/// Lower `fir.address_of` operation to `llvm.address_of` operation.
struct AddrOfOpConversion : public fir::FIROpConversion<fir::AddrOfOp> {
using FIROpConversion::FIROpConversion;
@@ -144,9 +176,15 @@ struct AddrOfOpConversion : public fir::FIROpConversion<fir::AddrOfOp> {
llvm::LogicalResult
matchAndRewrite(fir::AddrOfOp addr, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
- auto ty = convertType(addr.getType());
- rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
- addr, ty, addr.getSymbol().getRootReference().getValue());
+ auto global = addr->getParentOfType<mlir::ModuleOp>()
+ .lookupSymbol<mlir::LLVM::GlobalOp>(addr.getSymbol());
+ replaceWithAddrOfOrASCast(
+ rewriter, addr->getLoc(),
+ global ? global.getAddrSpace() : getGlobalAddressSpace(rewriter),
+ getProgramAddressSpace(rewriter),
+ global ? global.getSymName()
+ : addr.getSymbol().getRootReference().getValue(),
+ convertType(addr.getType()), addr);
return mlir::success();
}
};
@@ -1306,13 +1344,18 @@ getTypeDescriptor(ModOpTy mod, mlir::ConversionPatternRewriter &rewriter,
? fir::NameUniquer::getTypeDescriptorAssemblyName(recType.getName())
: fir::NameUniquer::getTypeDescriptorName(recType.getName());
mlir::Type llvmPtrTy = ::getLlvmPtrType(mod.getContext());
+ mlir::DataLayout dataLayout(mod);
if (auto global = mod.template lookupSymbol<fir::GlobalOp>(name))
- return rewriter.create<mlir::LLVM::AddressOfOp>(loc, llvmPtrTy,
- global.getSymName());
+ return replaceWithAddrOfOrASCast(
+ rewriter, loc, fir::factory::getGlobalAddressSpace(&dataLayout),
+ fir::factory::getProgramAddressSpace(&dataLayout), global.getSymName(),
+ llvmPtrTy);
// The global may have already been translated to LLVM.
if (auto global = mod.template lookupSymbol<mlir::LLVM::GlobalOp>(name))
- return rewriter.create<mlir::LLVM::AddressOfOp>(loc, llvmPtrTy,
- global.getSymName());
+ return replaceWithAddrOfOrASCast(
+ rewriter, loc, global.getAddrSpace(),
+ fir::factory::getProgramAddressSpace(&dataLayout), global.getSymName(),
+ llvmPtrTy);
// Type info derived types do not have type descriptors since they are the
// types defining type descriptors.
if (options.ignoreMissingTypeDescriptors ||
@@ -3130,8 +3173,8 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
mlir::SymbolRefAttr comdat;
llvm::ArrayRef<mlir::NamedAttribute> attrs;
auto g = rewriter.create<mlir::LLVM::GlobalOp>(
- loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0, 0,
- false, false, comdat, attrs, dbgExprs);
+ loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0,
+ getGlobalAddressSpace(rewriter), false, false, comdat, attrs, dbgExprs);
if (global.getAlignment() && *global.getAlignment() > 0)
g.setAlignment(*global.getAlignment());
diff --git a/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp b/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
index 12021deb4bd97..5c9c9c34caac1 100644
--- a/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
+++ b/flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp
@@ -365,4 +365,19 @@ unsigned ConvertFIRToLLVMPattern::getProgramAddressSpace(
return defaultAddressSpace;
}
+unsigned ConvertFIRToLLVMPattern::getGlobalAddressSpace(
+ mlir::ConversionPatternRewriter &rewriter) const {
+ mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
+ assert(parentOp != nullptr &&
+ "expected insertion block to have parent operation");
+ auto module = mlir::isa<mlir::ModuleOp>(parentOp)
+ ? mlir::cast<mlir::ModuleOp>(parentOp)
+ : parentOp->getParentOfType<mlir::ModuleOp>();
+ if (module)
+ if (mlir::Attribute addrSpace =
+ mlir::DataLayout(module).getGlobalMemorySpace())
+ return llvm::cast<mlir::IntegerAttr>(addrSpace).getUInt();
+ return defaultAddressSpace;
+}
+
} // namespace fir
diff --git a/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90 b/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90
new file mode 100644
index 0000000000000..249d403057b6c
--- /dev/null
+++ b/flang/test/Integration/amdgpu-target-desc-cast-to-global-addrspace.f90
@@ -0,0 +1,18 @@
+!REQUIRES: amdgpu-registered-target
+
+!RUN: %flang_fc1 -emit-llvm -triple amdgcn-amd-amdhsa -target-cpu gfx908 %s -o - | FileCheck %s
+
+subroutine maintest
+ implicit none
+
+ type r1_t
+ end type r1_t
+
+ type(r1_t), pointer :: A
+end subroutine
+
+! CHECK: @[[TYPE_DESC:.*XdtXr1_t]] = linkonce_odr addrspace(1) constant %_QM__fortran_type_infoTderivedtype
+
+! CHECK: define void @maintest_() {{.*}} {
+! CHECK: store { {{.*}} } { {{.*}}, ptr addrspacecast (ptr addrspace(1) @[[TYPE_DESC]] to ptr), {{.*}} }, {{.*}}
+! CHECK: }
|
if (dataLayout) | ||
if (mlir::Attribute addrSpace = dataLayout->getGlobalMemorySpace()) | ||
return mlir::cast<mlir::IntegerAttr>(addrSpace).getUInt(); | ||
return 0; |
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.
Why is there so much machinery? This should be direct read of constant
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.
This follow the same pattern as getAllocaAddressSpace
which I think was made this way to allow for testing modules that do not have data layout attached.
Added @tblah to verify if my understanding is correct or not (since it seems he wrote getAllocaAddressSpace
).
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.
Yes lots of flang lit tests do not have data layout strings. It would be inconvenient to add them.
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.
The changes look sensible enough to me, but I am not that familiar with how this change might effect architectures using these different address spaces.
There's another pretty much equivalent approved PR here: #119585. @agozillon let us know if you intend to merge that one soon or if we should look into getting this one landed instead. |
Thanks for bringing that to my attention, did not know about the PR. |
Feel free to land it! The PR stack that my version is tied to has been held up for a while due to some conflicts with the old variation of implicit mapping of allocatable derived type components; need to test it with Akash's new version, and I've been meaning to disentangle it for a while to land it separately but haven't found the time, so thank you for creating another PR :-) |
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, we have this downstream currently and it's yet to blow anything up after half a year-ish and it solves several issues, so it's a step in the right direction even if it may not be perfect (issues yet to be found :-) but someone with more address space knowledge may see issues).
Instead of emitting globals in the program/default address space, emit them in the global address space. This also requires changes how address of code-gen is handled, we need to cast to the default address space to prevent code-gen issues.