[flang-commits] [flang] 7ae3911 - [flang]Fix incorrect array type transformation
Mats Petersson via flang-commits
flang-commits at lists.llvm.org
Thu Jul 28 13:00:21 PDT 2022
Author: Mats Petersson
Date: 2022-07-28T21:00:04+01:00
New Revision: 7ae391148d6fba31fa4cb84678fe28ce6638e98c
URL: https://github.com/llvm/llvm-project/commit/7ae391148d6fba31fa4cb84678fe28ce6638e98c
DIFF: https://github.com/llvm/llvm-project/commit/7ae391148d6fba31fa4cb84678fe28ce6638e98c.diff
LOG: [flang]Fix incorrect array type transformation
When an array is defined with "unknown" size, such as fir.array<2x?x5xi32>,
it should be converted to llvm.array<10 x i32>. The code so far has
been converting it to llvm.ptr<i32>.
Using a different function to check the if there starting are constant
dimensions, rather than if ALL dimensions are constant, it now produces
the correct array form.
Some tests has been updated, so they are now checking the new behaviour
rather than the old behaviour - so there's no need to add further tests
for this particular scenario.
This was originally found when compiling Spec 17 code, where an assert
in a GepOP was hit. That is bug #56141, which this change fixes.
Reviewed By: jeanPerier
Differential Revision: https://reviews.llvm.org/D129196
Added:
Modified:
flang/include/flang/Lower/SymbolMap.h
flang/include/flang/Optimizer/Dialect/FIRType.h
flang/include/flang/Optimizer/Dialect/FIRTypes.td
flang/lib/Lower/ConvertExpr.cpp
flang/lib/Optimizer/CodeGen/CodeGen.cpp
flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
flang/lib/Optimizer/CodeGen/TypeConverter.h
flang/lib/Optimizer/Dialect/FIRType.cpp
flang/test/Fir/alloc.fir
flang/test/Fir/convert-to-llvm.fir
flang/test/Fir/types-to-llvm.fir
Removed:
################################################################################
diff --git a/flang/include/flang/Lower/SymbolMap.h b/flang/include/flang/Lower/SymbolMap.h
index 165776f3d7c0e..d2dd1bb29ce7b 100644
--- a/flang/include/flang/Lower/SymbolMap.h
+++ b/flang/include/flang/Lower/SymbolMap.h
@@ -145,7 +145,7 @@ struct SymbolBox : public fir::details::matcher<SymbolBox> {
bool hasConstantShape() const {
if (auto eleTy = fir::dyn_cast_ptrEleTy(getAddr().getType()))
if (auto arrTy = eleTy.dyn_cast<fir::SequenceType>())
- return arrTy.hasConstantShape();
+ return !arrTy.hasDynamicExtents();
return false;
}
diff --git a/flang/include/flang/Optimizer/Dialect/FIRType.h b/flang/include/flang/Optimizer/Dialect/FIRType.h
index 3fc25a330df26..e7726ef10e835 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRType.h
+++ b/flang/include/flang/Optimizer/Dialect/FIRType.h
@@ -155,7 +155,7 @@ inline bool characterWithDynamicLen(mlir::Type t) {
/// Returns true iff `seqTy` has either an unknown shape or a non-constant shape
/// (where rank > 0).
inline bool sequenceWithNonConstantShape(fir::SequenceType seqTy) {
- return seqTy.hasUnknownShape() || !seqTy.hasConstantShape();
+ return seqTy.hasUnknownShape() || seqTy.hasDynamicExtents();
}
/// Returns true iff the type `t` does not have a constant size.
diff --git a/flang/include/flang/Optimizer/Dialect/FIRTypes.td b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
index ac7505b9eb13c..2210dd00f8948 100644
--- a/flang/include/flang/Optimizer/Dialect/FIRTypes.td
+++ b/flang/include/flang/Optimizer/Dialect/FIRTypes.td
@@ -459,15 +459,12 @@ def fir_SequenceType : FIR_Type<"Sequence", "array"> {
// The number of dimensions of the sequence
unsigned getDimension() const { return getShape().size(); }
- // Is the interior of the sequence constant? Check if the array is
- // one of constant shape (`array<C...xCxT>`), unknown shape
- // (`array<*xT>`), or rows with shape and ending with column(s) of
- // unknown extent (`array<C...xCx?...x?xT>`).
- bool hasConstantInterior() const;
-
- // Is the shape of the sequence constant?
- bool hasConstantShape() const {
- return getConstantRows() == getDimension();
+ // Is the shape of the sequence dynamic?
+ bool hasDynamicExtents() const {
+ for(const auto d : getShape())
+ if (d == getUnknownExtent())
+ return true;
+ return false;
}
// Does the sequence have unknown shape? (`array<* x T>`)
diff --git a/flang/lib/Lower/ConvertExpr.cpp b/flang/lib/Lower/ConvertExpr.cpp
index f91662618a3c8..309fd4042f233 100644
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -4727,7 +4727,7 @@ class ArrayExprLowering {
fir::isRecordWithAllocatableMember(eleTy))
TODO(loc, "creating an array temp where the element type has "
"allocatable members");
- mlir::Value temp = seqTy.hasConstantShape()
+ mlir::Value temp = !seqTy.hasDynamicExtents()
? builder.create<fir::AllocMemOp>(loc, type)
: builder.create<fir::AllocMemOp>(
loc, type, ".array.expr", llvm::None, shape);
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 113556c280ba1..e1e59cf6428d8 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -332,20 +332,26 @@ genAllocationScaleSize(OP op, mlir::Type ity,
mlir::ConversionPatternRewriter &rewriter) {
mlir::Location loc = op.getLoc();
mlir::Type dataTy = op.getInType();
- mlir::Type scalarType = fir::unwrapSequenceType(dataTy);
auto seqTy = dataTy.dyn_cast<fir::SequenceType>();
- if ((op.hasShapeOperands() && seqTy && !seqTy.hasConstantInterior()) ||
- (seqTy && fir::characterWithDynamicLen(scalarType))) {
- fir::SequenceType::Extent constSize = 1;
- for (auto extent : seqTy.getShape())
- if (extent != fir::SequenceType::getUnknownExtent())
- constSize *= extent;
- if (constSize != 1) {
- mlir::Value constVal{
- genConstantIndex(loc, ity, rewriter, constSize).getResult()};
- return constVal;
+ fir::SequenceType::Extent constSize = 1;
+ if (seqTy) {
+ int constRows = seqTy.getConstantRows();
+ const fir::SequenceType::ShapeRef &shape = seqTy.getShape();
+ if (constRows != static_cast<int>(shape.size())) {
+ for (auto extent : shape) {
+ if (constRows-- > 0)
+ continue;
+ if (extent != fir::SequenceType::getUnknownExtent())
+ constSize *= extent;
+ }
}
}
+
+ if (constSize != 1) {
+ mlir::Value constVal{
+ genConstantIndex(loc, ity, rewriter, constSize).getResult()};
+ return constVal;
+ }
return nullptr;
}
diff --git a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
index 0abb9cafe7af2..96b9e9b41d93d 100644
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -81,7 +81,7 @@ class EmboxConversion : public mlir::OpRewritePattern<fir::EmboxOp> {
return rewriteDynamicShape(embox, rewriter, shapeVal);
if (auto boxTy = embox.getType().dyn_cast<fir::BoxType>())
if (auto seqTy = boxTy.getEleTy().dyn_cast<fir::SequenceType>())
- if (seqTy.hasConstantShape())
+ if (!seqTy.hasDynamicExtents())
return rewriteStaticShape(embox, rewriter, seqTy);
return mlir::failure();
}
diff --git a/flang/lib/Optimizer/CodeGen/TypeConverter.h b/flang/lib/Optimizer/CodeGen/TypeConverter.h
index 1c7ef9c0bc461..c087bdfda7366 100644
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.h
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.h
@@ -316,9 +316,9 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
// degenerate the array and do not want a the type to become `T**` but
// merely `T*`.
if (auto seqTy = eleTy.dyn_cast<fir::SequenceType>()) {
- if (!seqTy.hasConstantShape() ||
+ if (seqTy.hasDynamicExtents() ||
characterWithDynamicLen(seqTy.getEleTy())) {
- if (seqTy.hasConstantInterior())
+ if (seqTy.getConstantRows() > 0)
return convertType(seqTy);
eleTy = seqTy.getEleTy();
}
@@ -356,7 +356,7 @@ class LLVMTypeConverter : public mlir::LLVMTypeConverter {
if (--i == 0)
break;
}
- if (seq.hasConstantShape())
+ if (!seq.hasDynamicExtents())
return baseTy;
}
return mlir::LLVM::LLVMPointerType::get(baseTy);
diff --git a/flang/lib/Optimizer/Dialect/FIRType.cpp b/flang/lib/Optimizer/Dialect/FIRType.cpp
index 77075395576da..9f51121679d6b 100644
--- a/flang/lib/Optimizer/Dialect/FIRType.cpp
+++ b/flang/lib/Optimizer/Dialect/FIRType.cpp
@@ -783,33 +783,18 @@ void fir::SequenceType::print(mlir::AsmPrinter &printer) const {
}
unsigned fir::SequenceType::getConstantRows() const {
+ if (hasDynamicSize(getEleTy()))
+ return 0;
auto shape = getShape();
unsigned count = 0;
for (auto d : shape) {
- if (d < 0)
+ if (d == getUnknownExtent())
break;
++count;
}
return count;
}
-// This test helps us determine if we can degenerate an array to a
-// pointer to some interior section (possibly a single element) of the
-// sequence. This is used to determine if we can lower to the LLVM IR.
-bool fir::SequenceType::hasConstantInterior() const {
- if (hasUnknownShape())
- return true;
- auto rows = getConstantRows();
- auto dim = getDimension();
- if (rows == dim)
- return true;
- auto shape = getShape();
- for (unsigned i = rows, size = dim; i < size; ++i)
- if (shape[i] != getUnknownExtent())
- return false;
- return true;
-}
-
mlir::LogicalResult fir::SequenceType::verify(
llvm::function_ref<mlir::InFlightDiagnostic()> emitError,
llvm::ArrayRef<int64_t> shape, mlir::Type eleTy,
diff --git a/flang/test/Fir/alloc.fir b/flang/test/Fir/alloc.fir
index d3dddf70433ce..48cbd1ddebbf8 100644
--- a/flang/test/Fir/alloc.fir
+++ b/flang/test/Fir/alloc.fir
@@ -277,9 +277,9 @@ func.func @allocmem_dynarray_of_dynchar2(%l: i32, %e : index) -> !fir.heap<!fir.
// CHECK-LABEL: define ptr @alloca_array_with_holes_nonchar(
// CHECK-SAME: i64 %[[a:.*]], i64 %[[b:.*]])
-// CHECK: %[[prod1:.*]] = mul i64 60, %[[a]]
+// CHECK: %[[prod1:.*]] = mul i64 15, %[[a]]
// CHECK: %[[prod2:.*]] = mul i64 %[[prod1]], %[[b]]
-// CHECK: alloca i32, i64 %[[prod2]]
+// CHECK: alloca [4 x i32], i64 %[[prod2]]
func.func @alloca_array_with_holes_nonchar(%0 : index, %1 : index) -> !fir.ref<!fir.array<4x?x3x?x5xi32>> {
%a = fir.alloca !fir.array<4x?x3x?x5xi32>, %0, %1
return %a : !fir.ref<!fir.array<4x?x3x?x5xi32>>
@@ -287,8 +287,8 @@ func.func @alloca_array_with_holes_nonchar(%0 : index, %1 : index) -> !fir.ref<!
// CHECK-LABEL: define ptr @alloca_array_with_holes_char(
// CHECK-SAME: i64 %[[e:.*]])
-// CHECK: %[[mul:.*]] = mul i64 12, %[[e]]
-// CHECK: alloca [10 x i16], i64 %[[mul]]
+// CHECK: %[[mul:.*]] = mul i64 4, %[[e]]
+// CHECK: alloca [3 x [10 x i16]], i64 %[[mul]]
func.func @alloca_array_with_holes_char(%e: index) -> !fir.ref<!fir.array<3x?x4x!fir.char<2,10>>> {
%1 = fir.alloca !fir.array<3x?x4x!fir.char<2,10>>, %e
return %1 : !fir.ref<!fir.array<3x?x4x!fir.char<2,10>>>
@@ -306,7 +306,7 @@ func.func @alloca_array_with_holes_dynchar(%arg0: index, %arg1: index) -> !fir.r
// CHECK-LABEL: define ptr @allocmem_array_with_holes_nonchar(
// CHECK-SAME: i64 %[[e1:.*]], i64 %[[e2:.*]])
-// CHECK: %[[a:.*]] = mul i64 240, %[[e1]]
+// CHECK: %[[a:.*]] = mul i64 mul (i64 ptrtoint{{.*}} 15), %[[e1]]
// CHECK: %[[b:.*]] = mul i64 %3, %[[e2]]
// CHECK: call ptr @malloc(i64 %[[b]])
func.func @allocmem_array_with_holes_nonchar(%0 : index, %1 : index) -> !fir.heap<!fir.array<4x?x3x?x5xi32>> {
@@ -316,7 +316,7 @@ func.func @allocmem_array_with_holes_nonchar(%0 : index, %1 : index) -> !fir.hea
// CHECK-LABEL: define ptr @allocmem_array_with_holes_char(
// CHECK-SAME: i64 %[[e:.*]])
-// CHECK: %[[mul:.*]] = mul i64 mul (i64 ptrtoint (ptr getelementptr ([10 x i16], ptr null, i64 1) to i64), i64 12), %[[e]]
+// CHECK: %[[mul:.*]] = mul i64 mul (i64 ptrtoint (ptr getelementptr ([3 x [10 x i16]], ptr null, i64 1) to i64), i64 4), %[[e]]
// CHECK: call ptr @malloc(i64 %[[mul]])
func.func @allocmem_array_with_holes_char(%e: index) -> !fir.heap<!fir.array<3x?x4x!fir.char<2,10>>> {
%1 = fir.allocmem !fir.array<3x?x4x!fir.char<2,10>>, %e
diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir
index 7b5d1e9d326bc..3d37f62bfa164 100644
--- a/flang/test/Fir/convert-to-llvm.fir
+++ b/flang/test/Fir/convert-to-llvm.fir
@@ -1164,14 +1164,14 @@ func.func @alloca_array_with_holes(%0 : index, %1 : index) -> !fir.ref<!fir.arra
}
// CHECK-LABEL: llvm.func @alloca_array_with_holes
-// CHECK-SAME: ([[A:%.*]]: i64, [[B:%.*]]: i64) -> !llvm.ptr<i32>
+// CHECK-SAME: ([[A:%.*]]: i64, [[B:%.*]]: i64) -> !llvm.ptr<array<4 x i32>>
// CHECK-DAG: [[ONE:%.*]] = llvm.mlir.constant(1 : i64) : i64
-// CHECK-DAG: [[FIXED:%.*]] = llvm.mlir.constant(60 : i64) : i64
+// CHECK-DAG: [[FIXED:%.*]] = llvm.mlir.constant(15 : i64) : i64
// CHECK: [[PROD1:%.*]] = llvm.mul [[ONE]], [[FIXED]] : i64
// CHECK: [[PROD2:%.*]] = llvm.mul [[PROD1]], [[A]] : i64
// CHECK: [[PROD3:%.*]] = llvm.mul [[PROD2]], [[B]] : i64
-// CHECK: [[RES:%.*]] = llvm.alloca [[PROD3]] x i32 {in_type = !fir.array<4x?x3x?x5xi32>
-// CHECK: llvm.return [[RES]] : !llvm.ptr<i32>
+// CHECK: [[RES:%.*]] = llvm.alloca [[PROD3]] x !llvm.array<4 x i32> {in_type = !fir.array<4x?x3x?x5xi32>
+// CHECK: llvm.return [[RES]] : !llvm.ptr<array<4 x i32>>
// -----
diff --git a/flang/test/Fir/types-to-llvm.fir b/flang/test/Fir/types-to-llvm.fir
index 0754bdb826903..cd336ec181df3 100644
--- a/flang/test/Fir/types-to-llvm.fir
+++ b/flang/test/Fir/types-to-llvm.fir
@@ -49,7 +49,7 @@ func.func private @foo2(%arg0: !fir.ref<!fir.box<!fir.heap<!fir.type<_QMs1Ta1{x:
// CHECK-SAME: !llvm.ptr<struct<(ptr<struct<"_QMs1Ta1", (i32, f32)>>, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, i{{.*}}, ptr<i{{.*}}>, array<1 x i{{.*}}>)>>
func.func private @foo3(%arg0: !fir.ref<!fir.array<10x?x11xf32>>)
// CHECK-LABEL: foo3
-// CHECK-SAME: !llvm.ptr<f32>
+// CHECK-SAME: !llvm.ptr<array<10 x f32>>
func.func private @foo4(%arg0: !fir.ref<!fir.array<10x11x?x?xf32>>)
// CHECK-LABEL: foo4
// CHECK-SAME: !llvm.ptr<array<11 x array<10 x f32>>>
More information about the flang-commits
mailing list