[flang-commits] [flang] 88ac741 - [flang][hlfir] Fix hlfir.set_length codegen
Jean Perier via flang-commits
flang-commits at lists.llvm.org
Sun Apr 16 22:47:11 PDT 2023
Author: Jean Perier
Date: 2023-04-17T07:46:29+02:00
New Revision: 88ac7414d50b52b202a547e04a9f8bac4c797fa3
URL: https://github.com/llvm/llvm-project/commit/88ac7414d50b52b202a547e04a9f8bac4c797fa3
DIFF: https://github.com/llvm/llvm-project/commit/88ac7414d50b52b202a547e04a9f8bac4c797fa3.diff
LOG: [flang][hlfir] Fix hlfir.set_length codegen
The bufferization pass was propagating the raw alloca storage
(which may not allow to later retrieve the length) instead of
the hlfir variable value (which is guaranteed to hold the
character length).
Fix this and makes packageBufferizedExpr "storage" argument and
getBufferizedExprStorage return an hlfir::Entity to avoid similar
error in the future (the caller of packageBufferizedExpr will have
to think a bit when adding the explicit hlfir::Entity{} cast).
Differential Revision: https://reviews.llvm.org/D148307
Added:
Modified:
flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
flang/test/HLFIR/set_length-codegen.fir
Removed:
################################################################################
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
index 1f096efdeff41..a361208f8fef2 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
@@ -38,10 +38,12 @@ namespace hlfir {
namespace {
/// Helper to create tuple from a bufferized expr storage and clean up
-/// instruction flag.
+/// instruction flag. The storage is an HLFIR variable so that it can
+/// be manipulated as a variable later (all shape and length information
+/// cam be retrieved from it).
static mlir::Value packageBufferizedExpr(mlir::Location loc,
fir::FirOpBuilder &builder,
- mlir::Value storage,
+ hlfir::Entity storage,
mlir::Value mustFree) {
auto tupleType = mlir::TupleType::get(
builder.getContext(),
@@ -61,7 +63,7 @@ static mlir::Value packageBufferizedExpr(mlir::Location loc,
/// boolean clean-up flag.
static mlir::Value packageBufferizedExpr(mlir::Location loc,
fir::FirOpBuilder &builder,
- mlir::Value storage, bool mustFree) {
+ hlfir::Entity storage, bool mustFree) {
mlir::Value mustFreeValue = builder.createBool(loc, mustFree);
return packageBufferizedExpr(loc, builder, storage, mustFreeValue);
}
@@ -70,14 +72,14 @@ static mlir::Value packageBufferizedExpr(mlir::Location loc,
/// It assumes no tuples are used as HLFIR operation operands, which is
/// currently enforced by the verifiers that only accept HLFIR value or
/// variable types which do not include tuples.
-static mlir::Value getBufferizedExprStorage(mlir::Value bufferizedExpr) {
+static hlfir::Entity getBufferizedExprStorage(mlir::Value bufferizedExpr) {
auto tupleType = bufferizedExpr.getType().dyn_cast<mlir::TupleType>();
if (!tupleType)
- return bufferizedExpr;
+ return hlfir::Entity{bufferizedExpr};
assert(tupleType.size() == 2 && "unexpected tuple type");
if (auto insert = bufferizedExpr.getDefiningOp<fir::InsertValueOp>())
if (insert.getVal().getType() == tupleType.getType(0))
- return insert.getVal();
+ return hlfir::Entity{insert.getVal()};
TODO(bufferizedExpr.getLoc(), "general extract storage case");
}
@@ -152,7 +154,7 @@ struct AsExprOpConversion : public mlir::OpConversionPattern<hlfir::AsExprOp> {
if (asExpr.isMove()) {
// Move variable storage for the hlfir.expr buffer.
mlir::Value bufferizedExpr = packageBufferizedExpr(
- loc, builder, adaptor.getVar(), adaptor.getMustFree());
+ loc, builder, hlfir::Entity{adaptor.getVar()}, adaptor.getMustFree());
rewriter.replaceOp(asExpr, bufferizedExpr);
return mlir::success();
}
@@ -175,7 +177,7 @@ struct ApplyOpConversion : public mlir::OpConversionPattern<hlfir::ApplyOp> {
matchAndRewrite(hlfir::ApplyOp apply, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Location loc = apply->getLoc();
- hlfir::Entity bufferizedExpr{getBufferizedExprStorage(adaptor.getExpr())};
+ hlfir::Entity bufferizedExpr = getBufferizedExprStorage(adaptor.getExpr());
mlir::Type resultType = hlfir::getVariableElementType(bufferizedExpr);
mlir::Value result = rewriter.create<hlfir::DesignateOp>(
loc, resultType, bufferizedExpr, adaptor.getIndices(),
@@ -216,8 +218,8 @@ struct ConcatOpConversion : public mlir::OpConversionPattern<hlfir::ConcatOp> {
if (adaptor.getStrings().size() > 2)
TODO(loc, "codegen of optimized chained concatenation of more than two "
"strings");
- hlfir::Entity lhs{getBufferizedExprStorage(adaptor.getStrings()[0])};
- hlfir::Entity rhs{getBufferizedExprStorage(adaptor.getStrings()[1])};
+ hlfir::Entity lhs = getBufferizedExprStorage(adaptor.getStrings()[0]);
+ hlfir::Entity rhs = getBufferizedExprStorage(adaptor.getStrings()[1]);
auto [lhsExv, c1] = hlfir::translateToExtendedValue(loc, builder, lhs);
auto [rhsExv, c2] = hlfir::translateToExtendedValue(loc, builder, rhs);
assert(!c1 && !c2 && "expected variables");
@@ -229,9 +231,10 @@ struct ConcatOpConversion : public mlir::OpConversionPattern<hlfir::ConcatOp> {
hlfir::getFortranElementType(concat.getResult().getType()));
mlir::Value cast = builder.createConvert(loc, addrType, fir::getBase(res));
res = fir::substBase(res, cast);
- auto hlfirTempRes = hlfir::genDeclare(loc, builder, res, "tmp",
- fir::FortranVariableFlagsAttr{})
- .getBase();
+ hlfir::Entity hlfirTempRes =
+ hlfir::Entity{hlfir::genDeclare(loc, builder, res, "tmp",
+ fir::FortranVariableFlagsAttr{})
+ .getBase()};
mlir::Value bufferizedExpr =
packageBufferizedExpr(loc, builder, hlfirTempRes, false);
rewriter.replaceOp(concat, bufferizedExpr);
@@ -251,7 +254,7 @@ struct SetLengthOpConversion
auto module = setLength->getParentOfType<mlir::ModuleOp>();
fir::FirOpBuilder builder(rewriter, fir::getKindMapping(module));
// Create a temp with the new length.
- hlfir::Entity string{getBufferizedExprStorage(adaptor.getString())};
+ hlfir::Entity string = getBufferizedExprStorage(adaptor.getString());
auto charType = hlfir::getFortranElementType(setLength.getType());
llvm::StringRef tmpName{".tmp"};
llvm::SmallVector<mlir::Value, 1> lenParams{adaptor.getLength()};
@@ -260,10 +263,11 @@ struct SetLengthOpConversion
auto declareOp = builder.create<hlfir::DeclareOp>(
loc, alloca, tmpName, /*shape=*/mlir::Value{}, lenParams,
fir::FortranVariableFlagsAttr{});
+ hlfir::Entity temp{declareOp.getBase()};
// Assign string value to the created temp.
- builder.create<hlfir::AssignOp>(loc, string, declareOp.getBase());
+ builder.create<hlfir::AssignOp>(loc, string, temp);
mlir::Value bufferizedExpr =
- packageBufferizedExpr(loc, builder, alloca, false);
+ packageBufferizedExpr(loc, builder, temp, false);
rewriter.replaceOp(setLength, bufferizedExpr);
return mlir::success();
}
@@ -390,12 +394,12 @@ struct DestroyOpConversion
mlir::ConversionPatternRewriter &rewriter) const override {
// If expr was bufferized on the heap, now is time to deallocate the buffer.
mlir::Location loc = destroy->getLoc();
- mlir::Value bufferizedExpr = getBufferizedExprStorage(adaptor.getExpr());
+ hlfir::Entity bufferizedExpr = getBufferizedExprStorage(adaptor.getExpr());
if (!fir::isa_trivial(bufferizedExpr.getType())) {
auto module = destroy->getParentOfType<mlir::ModuleOp>();
fir::FirOpBuilder builder(rewriter, fir::getKindMapping(module));
mlir::Value mustFree = getBufferizedExprMustFreeFlag(adaptor.getExpr());
- mlir::Value firBase = hlfir::Entity(bufferizedExpr).getFirBase();
+ mlir::Value firBase = bufferizedExpr.getFirBase();
genFreeIfMustFree(loc, builder, firBase, mustFree);
}
rewriter.eraseOp(destroy);
diff --git a/flang/test/HLFIR/set_length-codegen.fir b/flang/test/HLFIR/set_length-codegen.fir
index a54cfc00ffe17..4e3c7f6136b48 100644
--- a/flang/test/HLFIR/set_length-codegen.fir
+++ b/flang/test/HLFIR/set_length-codegen.fir
@@ -15,7 +15,7 @@ func.func @test_cst_len(%str : !fir.boxchar<1>) {
// CHECK: %[[VAL_4:.*]] = arith.constant false
// CHECK: %[[VAL_5:.*]] = fir.undefined tuple<!fir.ref<!fir.char<1,10>>, i1>
// CHECK: %[[VAL_6:.*]] = fir.insert_value %[[VAL_5]], %[[VAL_4]], [1 : index] : (tuple<!fir.ref<!fir.char<1,10>>, i1>, i1) -> tuple<!fir.ref<!fir.char<1,10>>, i1>
-// CHECK: %[[VAL_7:.*]] = fir.insert_value %[[VAL_6]], %[[VAL_1]], [0 : index] : (tuple<!fir.ref<!fir.char<1,10>>, i1>, !fir.ref<!fir.char<1,10>>) -> tuple<!fir.ref<!fir.char<1,10>>, i1>
+// CHECK: %[[VAL_7:.*]] = fir.insert_value %[[VAL_6]], %[[VAL_3]]#0, [0 : index] : (tuple<!fir.ref<!fir.char<1,10>>, i1>, !fir.ref<!fir.char<1,10>>) -> tuple<!fir.ref<!fir.char<1,10>>, i1>
func.func @test_dyn_len(%str : !fir.ref<!fir.char<1,10>>, %len : index) {
%0 = hlfir.set_length %str len %len : (!fir.ref<!fir.char<1,10>>, index) -> !hlfir.expr<!fir.char<1,?>>
@@ -28,6 +28,6 @@ func.func @test_dyn_len(%str : !fir.ref<!fir.char<1,10>>, %len : index) {
// CHECK: %[[VAL_3:.*]]:2 = hlfir.declare %[[VAL_2]] typeparams %[[VAL_1]] {uniq_name = ".tmp"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>)
// CHECK: hlfir.assign %[[VAL_0]] to %[[VAL_3]]#0 : !fir.ref<!fir.char<1,10>>, !fir.boxchar<1>
// CHECK: %[[VAL_4:.*]] = arith.constant false
-// CHECK: %[[VAL_5:.*]] = fir.undefined tuple<!fir.ref<!fir.char<1,?>>, i1>
-// CHECK: %[[VAL_6:.*]] = fir.insert_value %[[VAL_5]], %[[VAL_4]], [1 : index] : (tuple<!fir.ref<!fir.char<1,?>>, i1>, i1) -> tuple<!fir.ref<!fir.char<1,?>>, i1>
-// CHECK: %[[VAL_7:.*]] = fir.insert_value %[[VAL_6]], %[[VAL_2]], [0 : index] : (tuple<!fir.ref<!fir.char<1,?>>, i1>, !fir.ref<!fir.char<1,?>>) -> tuple<!fir.ref<!fir.char<1,?>>, i1>
+// CHECK: %[[VAL_5:.*]] = fir.undefined tuple<!fir.boxchar<1>, i1>
+// CHECK: %[[VAL_6:.*]] = fir.insert_value %[[VAL_5]], %[[VAL_4]], [1 : index] : (tuple<!fir.boxchar<1>, i1>, i1) -> tuple<!fir.boxchar<1>, i1>
+// CHECK: %[[VAL_7:.*]] = fir.insert_value %[[VAL_6]], %[[VAL_3]]#0, [0 : index] : (tuple<!fir.boxchar<1>, i1>, !fir.boxchar<1>) -> tuple<!fir.boxchar<1>, i1>
More information about the flang-commits
mailing list