[flang-commits] [flang] b881fc2 - [flang] Fix array substring emboxing code generation
Jean Perier via flang-commits
flang-commits at lists.llvm.org
Thu Jun 29 09:40:28 PDT 2023
Author: Jean Perier
Date: 2023-06-29T18:40:01+02:00
New Revision: b881fc27378092989025f744124d19c1421af327
URL: https://github.com/llvm/llvm-project/commit/b881fc27378092989025f744124d19c1421af327
DIFF: https://github.com/llvm/llvm-project/commit/b881fc27378092989025f744124d19c1421af327.diff
LOG: [flang] Fix array substring emboxing code generation
The code generation of the fir.embox op creating descriptors for
array substring with a non constant length base was using the
substring length to compute the first dimension result stride.
Fix it to use the input length instead.
Reviewed By: PeteSteinfeld
Differential Revision: https://reviews.llvm.org/D154086
Added:
Modified:
flang/lib/Optimizer/CodeGen/CodeGen.cpp
flang/test/Fir/embox-substring.fir
Removed:
################################################################################
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 9934746e770cf..b03c5bb9c4218 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -1266,6 +1266,22 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
return CFI_attribute_other;
}
+ mlir::Value getCharacterByteSize(mlir::Location loc,
+ mlir::ConversionPatternRewriter &rewriter,
+ fir::CharacterType charTy,
+ mlir::ValueRange lenParams) const {
+ auto i64Ty = mlir::IntegerType::get(rewriter.getContext(), 64);
+ mlir::Value size =
+ genTypeStrideInBytes(loc, i64Ty, rewriter, this->convertType(charTy));
+ if (charTy.hasConstantLen())
+ return size; // Length accounted for in the genTypeStrideInBytes GEP.
+ // Otherwise, multiply the single character size by the length.
+ assert(!lenParams.empty());
+ auto len64 = FIROpConversion<OP>::integerCast(loc, rewriter, i64Ty,
+ lenParams.back());
+ return rewriter.create<mlir::LLVM::MulOp>(loc, i64Ty, size, len64);
+ }
+
// Get the element size and CFI type code of the boxed value.
std::tuple<mlir::Value, mlir::Value> getSizeAndTypeCode(
mlir::Location loc, mlir::ConversionPatternRewriter &rewriter,
@@ -1286,18 +1302,9 @@ struct EmboxCommonConversion : public FIROpConversion<OP> {
return {genTypeStrideInBytes(loc, i64Ty, rewriter,
this->convertType(boxEleTy)),
typeCodeVal};
- if (auto charTy = boxEleTy.dyn_cast<fir::CharacterType>()) {
- mlir::Value size =
- genTypeStrideInBytes(loc, i64Ty, rewriter, this->convertType(charTy));
- if (charTy.getLen() == fir::CharacterType::unknownLen()) {
- // Multiply the single character size by the length.
- assert(!lenParams.empty());
- auto len64 = FIROpConversion<OP>::integerCast(loc, rewriter, i64Ty,
- lenParams.back());
- size = rewriter.create<mlir::LLVM::MulOp>(loc, i64Ty, size, len64);
- }
- return {size, typeCodeVal};
- };
+ if (auto charTy = boxEleTy.dyn_cast<fir::CharacterType>())
+ return {getCharacterByteSize(loc, rewriter, charTy, lenParams),
+ typeCodeVal};
if (fir::isa_ref_type(boxEleTy)) {
auto ptrTy = mlir::LLVM::LLVMPointerType::get(
mlir::LLVM::LLVMVoidType::get(rewriter.getContext()));
@@ -1691,7 +1698,7 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
sourceBox = operands[xbox.getSourceBoxOffset()];
sourceBoxType = xbox.getSourceBox().getType();
}
- auto [boxTy, dest, eleSize] = consDescriptorPrefix(
+ auto [boxTy, dest, resultEleSize] = consDescriptorPrefix(
xbox, fir::unwrapRefType(xbox.getMemref().getType()), rewriter,
xbox.getOutRank(), adaptor.getSubstr(), adaptor.getLenParams(),
sourceBox, sourceBoxType);
@@ -1720,7 +1727,8 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
// Adjust the element scaling factor if the element is a dependent type.
if (fir::hasDynamicSize(seqEleTy)) {
if (auto charTy = seqEleTy.dyn_cast<fir::CharacterType>()) {
- prevPtrOff = eleSize;
+ prevPtrOff =
+ getCharacterByteSize(loc, rewriter, charTy, adaptor.getLenParams());
} else if (seqEleTy.isa<fir::RecordType>()) {
// prevPtrOff = ;
TODO(loc, "generate call to calculate size of PDT");
@@ -1734,8 +1742,10 @@ struct XEmboxOpConversion : public EmboxCommonConversion<fir::cg::XEmboxOp> {
const auto hasSubcomp = !xbox.getSubcomponent().empty();
const bool hasSubstr = !xbox.getSubstr().empty();
// Initial element stride that will be use to compute the step in
- // each dimension.
- mlir::Value prevDimByteStride = eleSize;
+ // each dimension. Initially, this is the size of the input element.
+ // Note that when there are no components/substring, the resultEleSize
+ // that was previously computed matches the input element size.
+ mlir::Value prevDimByteStride = resultEleSize;
if (hasSubcomp) {
// We have a subcomponent. The step value needs to be the number of
// bytes per element (which is a derived type).
diff --git a/flang/test/Fir/embox-substring.fir b/flang/test/Fir/embox-substring.fir
index 368c7bdb43ac9..fd0b5923df0ed 100644
--- a/flang/test/Fir/embox-substring.fir
+++ b/flang/test/Fir/embox-substring.fir
@@ -11,3 +11,28 @@ func.func @embox_index_substr(%addr : !fir.ref<!fir.array<?x!fir.char<1,2>>>) {
%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
}
+
+// CHARACTER(*) :: C(2)
+// CALL DUMP(C(:)(1:1))
+// Test that the resulting stride is based on the input length, not the substring one.
+func.func @substring_dyn_base(%base_addr: !fir.ref<!fir.array<2x!fir.char<1,?>>>, %base_len: index) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c2 = arith.constant 2 : index
+ %2 = fircg.ext_embox %base_addr(%c2)[%c1, %c2, %c1] substr %c0, %c1 typeparams %base_len : (!fir.ref<!fir.array<2x!fir.char<1,?>>>, index, index, index, index, index, index, index) -> !fir.box<!fir.array<2x!fir.char<1>>>
+ fir.call @dump(%2) : (!fir.box<!fir.array<2x!fir.char<1>>>) -> ()
+ return
+}
+func.func private @dump(!fir.box<!fir.array<2x!fir.char<1>>>)
+
+// CHECK-LABEL: llvm.func @substring_dyn_base(
+// CHECK-SAME: %[[VAL_0:.*]]: !llvm.ptr<i8>,
+// CHECK-SAME: %[[VAL_1:.*]]: i64) {
+// CHECK: %[[VAL_5:.*]] = llvm.mlir.constant(1 : index) : i64
+// CHECK: llvm.getelementptr
+// CHECK: %[[VAL_28:.*]] = llvm.mlir.null : !llvm.ptr<i8>
+// CHECK: %[[VAL_29:.*]] = llvm.getelementptr %[[VAL_28]][1] : (!llvm.ptr<i8>) -> !llvm.ptr<i8>
+// CHECK: %[[VAL_30:.*]] = llvm.ptrtoint %[[VAL_29]] : !llvm.ptr<i8> to i64
+// CHECK: %[[VAL_31:.*]] = llvm.mul %[[VAL_30]], %[[VAL_1]] : i64
+// CHECK: %[[VAL_42:.*]] = llvm.mul %[[VAL_31]], %[[VAL_5]] : i64
+// CHECK: %[[VAL_43:.*]] = llvm.insertvalue %[[VAL_42]], %{{.*}}[7, 0, 2] : !llvm.struct<(ptr<array<2 x array<1 x i8>>>, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>
More information about the flang-commits
mailing list