Skip to content

Commit

Permalink
[flang] Fixed substr access in embox/rebox CodeGen.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vzakhari committed Apr 18, 2023
1 parent 3950e44 commit a45ca5d
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
14 changes: 8 additions & 6 deletions flang/lib/Optimizer/CodeGen/CodeGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,7 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
std::tuple<fir::BaseBoxType, mlir::Value, mlir::Value>
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();
Expand All @@ -1454,7 +1455,7 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
llvm::SmallVector<mlir::Value> typeparams = lenParams;
if constexpr (!std::is_same_v<BOX, fir::EmboxOp>) {
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.
Expand Down Expand Up @@ -1486,14 +1487,15 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
std::tuple<fir::BaseBoxType, mlir::Value, mlir::Value>
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();
auto boxTy = box.getType().dyn_cast<fir::BaseBoxType>();
auto inputBoxTy = box.getBox().getType().dyn_cast<fir::BaseBoxType>();
llvm::SmallVector<mlir::Value> 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);
Expand Down Expand Up @@ -1660,8 +1662,8 @@ struct EmboxOpConversion : public EmboxCommonConversion<fir::EmboxOp> {
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(),
Expand Down Expand Up @@ -1691,7 +1693,7 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
}
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);
Expand Down Expand Up @@ -1914,7 +1916,7 @@ struct XReboxOpConversion : public EmboxCommonConversion<fir::cg::XReboxOp> {

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<mlir::Value> inputExtents;
Expand Down
13 changes: 13 additions & 0 deletions flang/test/Fir/embox-substring.fir
Original file line number Diff line number Diff line change
@@ -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<!fir.array<?x!fir.char<1,2>>>) {
%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.array<?x!fir.char<1,2>>>, !fir.shapeshift<1>, !fir.slice<1>) -> !fir.box<!fir.array<?x!fir.char<1,?>>>
return
}
20 changes: 20 additions & 0 deletions flang/test/Fir/rebox-susbtring.fir
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,23 @@ func.func @foo(%arg0: !fir.box<!fir.array<?x!fir.type<t{i:i32,c:!fir.char<1,10>}
fir.call @bar(%2) : (!fir.box<!fir.array<?x!fir.char<1,?>>>) -> ()
return
}

// Test that a rebox with `index` substring parameter is converted
// to legal IR. It used to produce:
// %63 = "llvm.mul"(%62, <<NULL VALUE>>) : (i64, <<NULL TYPE>>) -> 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<!fir.array<?x!fir.char<1,20>>>) {
%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<!fir.array<?x!fir.char<1,20>>>, 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.array<?x!fir.char<1,20>>>, !fir.slice<1>) -> !fir.box<!fir.array<?x!fir.char<1,?>>>
fir.call @bar(%2) : (!fir.box<!fir.array<?x!fir.char<1,?>>>) -> ()
return
}

0 comments on commit a45ca5d

Please sign in to comment.