[CIR][Lowering][MLIR] Rework the !cir.array lowering

Split completely the !cir.array lowering, like in struct/class/union, from any
reference with memref construction.
Rationalize the approach inside convertToReferenceType() instead of ad-hoc cases
all-over the place.
Fix a test which seems to have been wrong from the beginning.

Author: keryell

clang/lib/CIR/Lowering/ThroughMLIR/LowerCIRToMLIR.cpp

@@ -69,6 +69,55 @@ using namespace llvm;
 
 namespace cir {
 
+/// Given a type convertor and a data layout, convert the given type to a type
+/// that is suitable for memory operations. For example, this can be used to
+/// lower cir.bool accesses to i8.
+static mlir::Type convertTypeForMemory(const mlir::TypeConverter &converter,
+                                       mlir::Type type) {
+  // TODO(cir): Handle other types similarly to clang's codegen
+  // convertTypeForMemory
+  if (isa<cir::BoolType>(type)) {
+    // TODO: Use datalayout to get the size of bool
+    return mlir::IntegerType::get(type.getContext(), 8);
+  }
+
+  return converter.convertType(type);
+}
+
+// Create a reference to an MLIR type. This creates a memref of the element type
+// with the requested shape except when it is a tensor because it represents a
+// !cir.array which has to be blessed as a memref of the tensor element type
+// instead.
+static mlir::MemRefType convertToReferenceType(ArrayRef<int64_t> shape,
+                                               mlir::Type elementType) {
+  if (auto t = mlir::dyn_cast<mlir::TensorType>(elementType))
+    return mlir::MemRefType::get(t.getShape(), t.getElementType());
+  return mlir::MemRefType::get(shape, elementType);
+}
+
+// Lower a cir.array either as a memref when it has a reference semantics or as
+// a tensor when it has a value semantics (like inside a struct or union).
+mlir::Type lowerArrayType(cir::ArrayType type, bool hasValueSemantics,
+                          mlir::TypeConverter &converter) {
+  SmallVector<int64_t> shape;
+  mlir::Type curType = type;
+  while (auto arrayType = dyn_cast<cir::ArrayType>(curType)) {
+    shape.push_back(arrayType.getSize());
+    curType = arrayType.getEltType();
+  }
+  auto elementType = convertTypeForMemory(converter, curType);
+  // FIXME: The element type might not be converted.
+  if (!elementType)
+    return nullptr;
+  // Arrays in C/C++ have a value  semantics when in a struct, so use
+  // a tensor.
+  // TODO: tensors cannot contain most built-in types like memref.
+  if (hasValueSemantics)
+    return mlir::RankedTensorType::get(shape, elementType);
+  // Otherwise, go to a memref.
+  return convertToReferenceType(shape, elementType);
+}
+
 class CIRReturnLowering : public mlir::OpConversionPattern<cir::ReturnOp> {
 public:
   using OpConversionPattern<cir::ReturnOp>::OpConversionPattern;
@@ -121,21 +170,6 @@ class CIRCallOpLowering : public mlir::OpConversionPattern<cir::CallOp> {
   }
 };
 
-/// Given a type convertor and a data layout, convert the given type to a type
-/// that is suitable for memory operations. For example, this can be used to
-/// lower cir.bool accesses to i8.
-static mlir::Type convertTypeForMemory(const mlir::TypeConverter &converter,
-                                       mlir::Type type) {
-  // TODO(cir): Handle other types similarly to clang's codegen
-  // convertTypeForMemory
-  if (isa<cir::BoolType>(type)) {
-    // TODO: Use datalayout to get the size of bool
-    return mlir::IntegerType::get(type.getContext(), 8);
-  }
-
-  return converter.convertType(type);
-}
-
 /// Emits the value from memory as expected by its users. Should be called when
 /// the memory represetnation of a CIR type is not equal to its scalar
 /// representation.
@@ -184,14 +218,7 @@ class CIRAllocaOpLowering : public mlir::OpConversionPattern<cir::AllocaOp> {
     if (!mlirType)
       return mlir::LogicalResult::failure();
 
-    auto memreftype = mlir::dyn_cast<mlir::MemRefType>(mlirType);
-    if (memreftype && mlir::isa<cir::ArrayType>(adaptor.getAllocaType())) {
-      // if the type is an array,
-      // we don't need to wrap with memref.
-    } else {
-      memreftype = mlir::MemRefType::get({}, mlirType);
-    }
-
+    auto memreftype = convertToReferenceType({}, mlirType);
     rewriter.replaceOpWithNewOp<mlir::memref::AllocaOp>(op, memreftype,
                                                         op.getAlignmentAttr());
     return mlir::LogicalResult::success();
@@ -333,7 +360,7 @@ class CIRGetMemberOpLowering
     auto flattenedMemRef = mlir::MemRefType::get(
         linearizedSize, mlir::IntegerType::get(getContext(), 8));
     // Use a special cast because normal memref cast cannot do such an extreme
-    // cast.
+    // cast. Could be an UnrealizedCastOp instead?
     auto bytesMemRef = rewriter.create<mlir::named_tuple::CastOp>(
         op.getLoc(), mlir::TypeRange{flattenedMemRef},
         mlir::ValueRange{adaptor.getAddr()});
@@ -949,7 +976,7 @@ class CIRGlobalOpLowering : public mlir::OpConversionPattern<cir::GlobalOp> {
       return mlir::failure();
     auto memrefType = dyn_cast<mlir::MemRefType>(convertedType);
     if (!memrefType)
-      memrefType = mlir::MemRefType::get({}, convertedType);
+      memrefType = convertToReferenceType({}, convertedType);
     // Add an optional alignment to the global memref.
     mlir::IntegerAttr memrefAlignment =
         op.getAlignment()
@@ -1394,27 +1421,6 @@ void populateCIRToMLIRConversionPatterns(mlir::RewritePatternSet &patterns,
                                        cirDataLayout);
 }
 
-// Lower a cir.array either as a memref when it has a reference semantics or as
-// a tensor when it has a value semantics (like inside a struct or union)
-mlir::Type lowerArrayType(cir::ArrayType type, bool hasValueSemantics,
-                          mlir::TypeConverter &converter) {
-  SmallVector<int64_t> shape;
-  mlir::Type curType = type;
-  while (auto arrayType = dyn_cast<cir::ArrayType>(curType)) {
-    shape.push_back(arrayType.getSize());
-    curType = arrayType.getEltType();
-  }
-  auto elementType = convertTypeForMemory(converter, curType);
-  // FIXME: The element type might not be converted
-  if (!elementType)
-    return nullptr;
-  // Arrays in C/C++ have a reference semantics when not in a struct, so use
-  // a memref
-  if (hasValueSemantics)
-    return mlir::RankedTensorType::get(shape, elementType);
-  return mlir::MemRefType::get(shape, elementType);
-}
-
 mlir::TypeConverter prepareTypeConverter(mlir::DataLayout &dataLayout) {
   mlir::TypeConverter converter;
   converter.addConversion([&](cir::PointerType type) -> mlir::Type {
@@ -1423,12 +1429,8 @@ mlir::TypeConverter prepareTypeConverter(mlir::DataLayout &dataLayout) {
                                        // converted (e.g. struct)
     if (!ty)
       return nullptr;
-    if (isa<cir::ArrayType>(type.getPointee()))
-      // An array is already lowered as a memref with reference semantics by
-      // default
-      return ty;
     // Each level of pointer becomes a level of memref
-    return mlir::MemRefType::get({}, ty);
+    return convertToReferenceType({}, ty);
   });
   converter.addConversion(
       [&](mlir::IntegerType type) -> mlir::Type { return type; });
@@ -1466,23 +1468,15 @@ mlir::TypeConverter prepareTypeConverter(mlir::DataLayout &dataLayout) {
     return mlir::BFloat16Type::get(type.getContext());
   });
   converter.addConversion([&](cir::ArrayType type) -> mlir::Type {
-    // Assume we are not in a class/struct/union context with value semantics,
-    // so lower it as a memref to provide reference semantics.
-    return lowerArrayType(type, /* hasValueSemantics */ false, converter);
+    // Assume we are in a class/struct/union context with value semantics,
+    // so lower it as a tensor to provide value semantics.
+    return lowerArrayType(type, /* hasValueSemantics */ true, converter);
   });
   converter.addConversion([&](cir::VectorType type) -> mlir::Type {
     auto ty = converter.convertType(type.getEltType());
     return mlir::VectorType::get(type.getSize(), ty);
   });
   converter.addConversion([&](cir::StructType type) -> mlir::Type {
-    auto convertWithValueSemanticsArray = [&](mlir::Type t) {
-      if (mlir::isa<cir::ArrayType>(t))
-        // Inside a class/struct/union, an array has value semantics and is
-        // lowered as a tensor.
-        return lowerArrayType(mlir::cast<cir::ArrayType>(t),
-                              /* hasValueSemantics */ true, converter);
-      return converter.convertType(t);
-    };
     // FIXME(cir): create separate unions, struct, and classes types.
     // Convert struct members.
     llvm::SmallVector<mlir::Type> mlirMembers;
@@ -1491,13 +1485,13 @@ mlir::TypeConverter prepareTypeConverter(mlir::DataLayout &dataLayout) {
       // TODO(cir): This should be properly validated.
     case cir::StructType::Struct:
       for (auto ty : type.getMembers())
-        mlirMembers.push_back(convertWithValueSemanticsArray(ty));
+        mlirMembers.push_back(converter.convertType(ty));
       break;
     // Unions are lowered as only the largest member.
     case cir::StructType::Union: {
       auto largestMember = type.getLargestMember(dataLayout);
       if (largestMember)
-        mlirMembers.push_back(convertWithValueSemanticsArray(largestMember));
+        mlirMembers.push_back(converter.convertType(largestMember));
       break;
     }
     }