[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

diff --git a/flang/include/flang/Lower/SymbolMap.h b/flang/include/flang/Lower/SymbolMap.h
index 165776f..d2dd1bb 100644 (file)
--- 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 3fc25a3..e7726ef 100644 (file)
--- 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 ac7505b..2210dd0 100644 (file)
--- 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 f916626..309fd40 100644 (file)
--- a/flang/lib/Lower/ConvertExpr.cpp
+++ b/flang/lib/Lower/ConvertExpr.cpp
@@ -4727,7 +4727,7 @@ private:
         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 113556c..e1e59cf 100644 (file)
--- 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 0abb9ca..96b9e9b 100644 (file)
--- a/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/PreCGRewrite.cpp
@@ -81,7 +81,7 @@ public:
       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 1c7ef9c..c087bdf 100644 (file)
--- a/flang/lib/Optimizer/CodeGen/TypeConverter.h
+++ b/flang/lib/Optimizer/CodeGen/TypeConverter.h
@@ -316,9 +316,9 @@ public:
     // 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 @@ public:
         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 7707539..9f51121 100644 (file)
--- 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 d3dddf7..48cbd1d 100644 (file)
--- 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 7b5d1e9..3d37f62 100644 (file)
--- 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 0754bdb..cd336ec 100644 (file)
--- 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>>>