[flang-commits] [flang] 9e1395e - [flang] Make a descriptor copy for fir.load fir.ref<fir.box>
Jean Perier via flang-commits
flang-commits at lists.llvm.org
Tue Sep 13 23:56:56 PDT 2022
Author: Jean Perier
Date: 2022-09-14T08:56:33+02:00
New Revision: 9e1395ef14144a6cde5255f7dec9d72b6ca74e4b
URL: https://github.com/llvm/llvm-project/commit/9e1395ef14144a6cde5255f7dec9d72b6ca74e4b
DIFF: https://github.com/llvm/llvm-project/commit/9e1395ef14144a6cde5255f7dec9d72b6ca74e4b.diff
LOG: [flang] Make a descriptor copy for fir.load fir.ref<fir.box>
`fir.box` and `fir.ref<fir.box>` are both lowered to LLVM as a
descriptor in memory. This is because fir.box of polymorphic and assumed
rank entities cannot be known at compile time, so fir.box cannot be
lowered to a struct value.
fir.load or fir.ref<fir.box> was previously lowered to a no-op,
propagating the operand descriptor storage as a result.
This is wrong because the operand descriptor storage may later be
modified, and these changes should not be visible in the loaded fir.box
that is an immutable SSA value.
Modify fir.load codegen for fir.box to make a copy into a new storage to
ensure the fir.box is immutable.
Differential Revision: https://reviews.llvm.org/D133779
Added:
Modified:
flang/lib/Optimizer/CodeGen/CodeGen.cpp
flang/test/Fir/convert-to-llvm.fir
Removed:
################################################################################
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 42472d2efc472..783bbf68b908a 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -263,6 +263,31 @@ class FIROpConversion : public mlir::ConvertOpToLLVMPattern<FromOp> {
return rewriter.create<mlir::LLVM::GEPOp>(loc, ty, base, cv);
}
+ // Find the LLVMFuncOp in whose entry block the alloca should be inserted.
+ // The order to find the LLVMFuncOp is as follows:
+ // 1. The parent operation of the current block if it is a LLVMFuncOp.
+ // 2. The first ancestor that is a LLVMFuncOp.
+ mlir::LLVM::LLVMFuncOp
+ getFuncForAllocaInsert(mlir::ConversionPatternRewriter &rewriter) const {
+ mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
+ return mlir::isa<mlir::LLVM::LLVMFuncOp>(parentOp)
+ ? mlir::cast<mlir::LLVM::LLVMFuncOp>(parentOp)
+ : parentOp->getParentOfType<mlir::LLVM::LLVMFuncOp>();
+ }
+
+ // Generate an alloca of size 1 and type \p toTy.
+ mlir::LLVM::AllocaOp
+ genAllocaWithType(mlir::Location loc, mlir::Type toTy, unsigned alignment,
+ mlir::ConversionPatternRewriter &rewriter) const {
+ auto thisPt = rewriter.saveInsertionPoint();
+ mlir::LLVM::LLVMFuncOp func = getFuncForAllocaInsert(rewriter);
+ rewriter.setInsertionPointToStart(&func.front());
+ auto size = genI32Constant(loc, rewriter, 1);
+ auto al = rewriter.create<mlir::LLVM::AllocaOp>(loc, toTy, size, alignment);
+ rewriter.restoreInsertionPoint(thisPt);
+ return al;
+ }
+
fir::LLVMTypeConverter &lowerTy() const {
return *static_cast<fir::LLVMTypeConverter *>(this->getTypeConverter());
}
@@ -1096,31 +1121,6 @@ template <typename OP>
struct EmboxCommonConversion : public FIROpConversion<OP> {
using FIROpConversion<OP>::FIROpConversion;
- // Find the LLVMFuncOp in whose entry block the alloca should be inserted.
- // The order to find the LLVMFuncOp is as follows:
- // 1. The parent operation of the current block if it is a LLVMFuncOp.
- // 2. The first ancestor that is a LLVMFuncOp.
- mlir::LLVM::LLVMFuncOp
- getFuncForAllocaInsert(mlir::ConversionPatternRewriter &rewriter) const {
- mlir::Operation *parentOp = rewriter.getInsertionBlock()->getParentOp();
- return mlir::isa<mlir::LLVM::LLVMFuncOp>(parentOp)
- ? mlir::cast<mlir::LLVM::LLVMFuncOp>(parentOp)
- : parentOp->getParentOfType<mlir::LLVM::LLVMFuncOp>();
- }
-
- // Generate an alloca of size 1 and type \p toTy.
- mlir::LLVM::AllocaOp
- genAllocaWithType(mlir::Location loc, mlir::Type toTy, unsigned alignment,
- mlir::ConversionPatternRewriter &rewriter) const {
- auto thisPt = rewriter.saveInsertionPoint();
- mlir::LLVM::LLVMFuncOp func = getFuncForAllocaInsert(rewriter);
- rewriter.setInsertionPointToStart(&func.front());
- auto size = this->genI32Constant(loc, rewriter, 1);
- auto al = rewriter.create<mlir::LLVM::AllocaOp>(loc, toTy, size, alignment);
- rewriter.restoreInsertionPoint(thisPt);
- return al;
- }
-
static int getCFIAttr(fir::BoxType boxTy) {
auto eleTy = boxTy.getEleTy();
if (eleTy.isa<fir::PointerType>())
@@ -1474,7 +1474,8 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
if (thisBlock && mlir::isa<mlir::LLVM::GlobalOp>(thisBlock->getParentOp()))
return boxValue;
auto boxPtrTy = mlir::LLVM::LLVMPointerType::get(boxValue.getType());
- auto alloca = genAllocaWithType(loc, boxPtrTy, defaultAlign, rewriter);
+ auto alloca =
+ this->genAllocaWithType(loc, boxPtrTy, defaultAlign, rewriter);
rewriter.create<mlir::LLVM::StoreOp>(loc, boxValue, alloca);
return alloca;
}
@@ -2714,12 +2715,29 @@ struct LoadOpConversion : public FIROpConversion<fir::LoadOp> {
mlir::LogicalResult
matchAndRewrite(fir::LoadOp load, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
- // fir.box is a special case because it is considered as an ssa values in
- // fir, but it is lowered as a pointer to a descriptor. So fir.ref<fir.box>
- // and fir.box end up being the same llvm types and loading a
- // fir.ref<fir.box> is actually a no op in LLVM.
- if (load.getType().isa<fir::BoxType>()) {
- rewriter.replaceOp(load, adaptor.getOperands()[0]);
+ if (auto boxTy = load.getType().dyn_cast<fir::BoxType>()) {
+ // fir.box is a special case because it is considered as an ssa values in
+ // fir, but it is lowered as a pointer to a descriptor. So
+ // fir.ref<fir.box> and fir.box end up being the same llvm types and
+ // loading a fir.ref<fir.box> is implemented as taking a snapshot of the
+ // descriptor value into a new descriptor temp.
+ auto inputBoxStorage = adaptor.getOperands()[0];
+ mlir::Location loc = load.getLoc();
+ fir::SequenceType seqTy = fir::unwrapUntilSeqType(boxTy);
+ mlir::Type eleTy = fir::unwrapPassByRefType(boxTy);
+ // fir.box of assumed rank and polymorphic entities do not have a storage
+ // size that is know at compile time. The copy needs to be runtime driven
+ // depending on the actual dynamic rank or type.
+ if (eleTy.isa<mlir::NoneType>() || (seqTy && seqTy.hasUnknownShape()))
+ TODO(loc, "loading polymorphic or assumed rank fir.box");
+ mlir::Type boxPtrTy = inputBoxStorage.getType();
+ auto boxValue = rewriter.create<mlir::LLVM::LoadOp>(
+ loc, boxPtrTy.cast<mlir::LLVM::LLVMPointerType>().getElementType(),
+ inputBoxStorage);
+ auto newBoxStorage =
+ genAllocaWithType(loc, boxPtrTy, defaultAlign, rewriter);
+ rewriter.create<mlir::LLVM::StoreOp>(loc, boxValue, newBoxStorage);
+ rewriter.replaceOp(load, newBoxStorage.getResult());
} else {
rewriter.replaceOpWithNewOp<mlir::LLVM::LoadOp>(
load, convertType(load.getType()), adaptor.getOperands(),
diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir
index a7630d7c7353e..883374a2066ab 100644
--- a/flang/test/Fir/convert-to-llvm.fir
+++ b/flang/test/Fir/convert-to-llvm.fir
@@ -867,15 +867,23 @@ func.func @test_load_index(%addr : !fir.ref<index>) {
// CHECK-NEXT: llvm.return
// CHECK-NEXT: }
+func.func private @takes_box(!fir.box<!fir.array<10xf32>>) -> ()
+
func.func @test_load_box(%addr : !fir.ref<!fir.box<!fir.array<10xf32>>>) {
%0 = fir.load %addr : !fir.ref<!fir.box<!fir.array<10xf32>>>
+ fir.call @takes_box(%0) : (!fir.box<!fir.array<10xf32>>) -> ()
return
}
-// Loading a `fir.ref<!fir.box>> is a no-op
-// CHECK-LABEL: llvm.func @test_load_box
-// CHECK-SAME: (%{{.*}}: !llvm.ptr<struct<(ptr<array<10 x f{{.*}}>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, array<1 x array<3 x i{{.*}}>>)>>) {
-// CHECK-NEXT: llvm.return
+// Loading a `fir.ref<!fir.box>> is creating a descriptor copy
+// CHECK-LABEL: llvm.func @test_load_box(
+// CHECK-SAME: %[[arg0:.*]]: !llvm.ptr<struct<([[DESC_TYPE:.*]])>>) {
+// CHECK-NEXT: %[[c1:.*]] = llvm.mlir.constant(1 : i32) : i32
+// CHECK-NEXT: %[[box_copy:.*]] = llvm.alloca %[[c1]] x !llvm.struct<([[DESC_TYPE]])>
+// CHECK-NEXT: %[[box_val:.*]] = llvm.load %[[arg0]] : !llvm.ptr<struct<([[DESC_TYPE]])>>
+// CHECK-NEXT: llvm.store %[[box_val]], %[[box_copy]] : !llvm.ptr<struct<([[DESC_TYPE]])>>
+// CHECK-NEXT: llvm.call @takes_box(%[[box_copy]]) : (!llvm.ptr<struct<([[DESC_TYPE]])>>) -> ()
+// CHECK-NEXT: llvm.return
// CHECK-NEXT: }
// -----
More information about the flang-commits
mailing list