From a45ca5d999dc4631f4680febe99ce49df3e813b1 Mon Sep 17 00:00:00 2001 From: Slava Zakharin Date: Mon, 17 Apr 2023 17:06:14 -0700 Subject: [PATCH] [flang] Fixed substr access in embox/rebox CodeGen. The code was using the original operand of the operation, while it should have been using the remapped operands via the adaptor. Differential Revision: https://reviews.llvm.org/D148587 --- flang/lib/Optimizer/CodeGen/CodeGen.cpp | 14 ++++++++------ flang/test/Fir/embox-substring.fir | 13 +++++++++++++ flang/test/Fir/rebox-susbtring.fir | 20 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 flang/test/Fir/embox-substring.fir diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp index 058cb70..2e461d7 100644 --- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp +++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp @@ -1445,6 +1445,7 @@ struct EmboxCommonConversion : public FIROpConversion { std::tuple consDescriptorPrefix(BOX box, mlir::Type inputType, mlir::ConversionPatternRewriter &rewriter, unsigned rank, + [[maybe_unused]] mlir::ValueRange substrParams, mlir::ValueRange lenParams, mlir::Value sourceBox = {}, mlir::Type sourceBoxType = {}) const { auto loc = box.getLoc(); @@ -1454,7 +1455,7 @@ struct EmboxCommonConversion : public FIROpConversion { llvm::SmallVector typeparams = lenParams; if constexpr (!std::is_same_v) { if (!box.getSubstr().empty() && fir::hasDynamicSize(boxTy.getEleTy())) - typeparams.push_back(box.getSubstr()[1]); + typeparams.push_back(substrParams[1]); } // Write each of the fields with the appropriate values. @@ -1486,6 +1487,7 @@ struct EmboxCommonConversion : public FIROpConversion { std::tuple consDescriptorPrefix(fir::cg::XReboxOp box, mlir::Value loweredBox, mlir::ConversionPatternRewriter &rewriter, unsigned rank, + mlir::ValueRange substrParams, mlir::ValueRange lenParams, mlir::Value typeDesc = {}) const { auto loc = box.getLoc(); @@ -1493,7 +1495,7 @@ struct EmboxCommonConversion : public FIROpConversion { auto inputBoxTy = box.getBox().getType().dyn_cast(); llvm::SmallVector typeparams = lenParams; if (!box.getSubstr().empty() && fir::hasDynamicSize(boxTy.getEleTy())) - typeparams.push_back(box.getSubstr()[1]); + typeparams.push_back(substrParams[1]); auto [eleSize, cfiTy] = getSizeAndTypeCode(loc, rewriter, boxTy.getEleTy(), typeparams); @@ -1660,8 +1662,8 @@ struct EmboxOpConversion : public EmboxCommonConversion { assert(!embox.getShape() && "There should be no dims on this embox op"); auto [boxTy, dest, eleSize] = consDescriptorPrefix( embox, fir::unwrapRefType(embox.getMemref().getType()), rewriter, - /*rank=*/0, /*lenParams=*/operands.drop_front(1), sourceBox, - sourceBoxType); + /*rank=*/0, /*substrParams=*/mlir::ValueRange{}, + adaptor.getTypeparams(), sourceBox, sourceBoxType); dest = insertBaseAddress(rewriter, embox.getLoc(), dest, operands[0]); if (fir::isDerivedTypeWithLenParams(boxTy)) { TODO(embox.getLoc(), @@ -1691,7 +1693,7 @@ struct XEmboxOpConversion : public EmboxCommonConversion { } auto [boxTy, dest, eleSize] = consDescriptorPrefix( xbox, fir::unwrapRefType(xbox.getMemref().getType()), rewriter, - xbox.getOutRank(), operands.drop_front(xbox.lenParamOffset()), + xbox.getOutRank(), adaptor.getSubstr(), adaptor.getLenParams(), sourceBox, sourceBoxType); // Generate the triples in the dims field of the descriptor auto i64Ty = mlir::IntegerType::get(xbox.getContext(), 64); @@ -1914,7 +1916,7 @@ struct XReboxOpConversion : public EmboxCommonConversion { auto [boxTy, dest, eleSize] = consDescriptorPrefix(rebox, loweredBox, rewriter, rebox.getOutRank(), - lenParams, typeDescAddr); + adaptor.getSubstr(), lenParams, typeDescAddr); // Read input extents, strides, and base address llvm::SmallVector inputExtents; diff --git a/flang/test/Fir/embox-substring.fir b/flang/test/Fir/embox-substring.fir new file mode 100644 index 0000000..368c7bd --- /dev/null +++ b/flang/test/Fir/embox-substring.fir @@ -0,0 +1,13 @@ +// RUN: fir-opt -o - -cg-rewrite --fir-to-llvm-ir %s | FileCheck %s +// RUN: tco -o - -cg-rewrite --fir-to-llvm-ir %s | FileCheck %s + +// CHECK-LABEL: llvm.func @embox_index_substr( +// CHECK-NOT: NULL_VALUE +// CHECK-NOT: NULL_TYPE +func.func @embox_index_substr(%addr : !fir.ref>>) { + %0 = arith.constant 0 : index + %1 = fir.shape_shift %0, %0 : (index, index) -> !fir.shapeshift<1> + %2 = fir.slice %0, %0, %0 substr %0, %0: (index, index, index, index, index) -> !fir.slice<1> + %3 = fir.embox %addr (%1) [%2] : (!fir.ref>>, !fir.shapeshift<1>, !fir.slice<1>) -> !fir.box>> + return +} diff --git a/flang/test/Fir/rebox-susbtring.fir b/flang/test/Fir/rebox-susbtring.fir index 70df4df..e2ca1b0 100644 --- a/flang/test/Fir/rebox-susbtring.fir +++ b/flang/test/Fir/rebox-susbtring.fir @@ -65,3 +65,23 @@ func.func @foo(%arg0: !fir.box} fir.call @bar(%2) : (!fir.box>>) -> () return } + +// Test that a rebox with `index` substring parameter is converted +// to legal IR. It used to produce: +// %63 = "llvm.mul"(%62, <>) : (i64, <>) -> i64 +// because the substr was not accessed via the adaptor's operands. + +// CHECK-LABEL: llvm.func @index_substr( +// CHECK-NOT: NULL_VALUE +// CHECK-NOT: NULL_TYPE +func.func @index_substr(%arg0: !fir.box>>) { + %c7_index = arith.constant 7 : index + %c1_i64 = arith.constant 1 : i64 + %c0 = arith.constant 0 : index + %c1 = arith.constant 1 : index + %0:3 = fir.box_dims %arg0, %c0 : (!fir.box>>, index) -> (index, index, index) + %1 = fir.slice %c1, %0#1, %c1_i64 substr %c1_i64, %c7_index : (index, index, i64, i64, index) -> !fir.slice<1> + %2 = fir.rebox %arg0 [%1] : (!fir.box>>, !fir.slice<1>) -> !fir.box>> + fir.call @bar(%2) : (!fir.box>>) -> () + return +} -- 2.7.4