[CIR][CIRGen][NFC] Refactor StructType builders

Instead of using a single builder for every possible StructType, we now have three builders: identified complete, identified incomplete, and anonymous struct types. This allows us to enforce correctness and to explicitly show the intent when creating a StructType.
llvm · Oct 27, 2023 · fb7508b · fb7508b
1 parent 9a33d14
commit fb7508b
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 32 deletions.
diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td
@@ -115,6 +115,37 @@ def CIR_StructType : CIR_Type<"Struct", "struct",
     "ASTRecordDeclInterface":$ast
   );
 
+  let skipDefaultBuilders = 1;
+  let builders = [
+    // Build an identified and complete struct.
+    TypeBuilder<(ins
+      "ArrayRef<Type>":$members,
+      "StringAttr":$name,
+      "bool":$packed,
+      "RecordKind":$kind,
+      CArg<"ASTRecordDeclInterface", "nullptr">:$ast), [{
+      return $_get(context, members, name, /*incomplete=*/false,
+                   packed, kind, ast);
+    }]>,
+    // Build an incomplete struct.
+    TypeBuilder<(ins
+      "StringAttr":$name,
+      "RecordKind":$kind), [{
+      return $_get(context, /*members=*/ArrayRef<Type>{}, name,
+                   /*incomplete=*/true, /*packed=*/false, kind,
+                   /*ast=*/nullptr);
+    }]>,
+    // Build an anonymous struct.
+    TypeBuilder<(ins
+      "ArrayRef<Type>":$members,
+      "bool":$packed,
+      "RecordKind":$kind,
+      CArg<"ASTRecordDeclInterface", "nullptr">:$ast), [{
+      return $_get(context, members, /*name=*/nullptr,
+                   /*incomplete=*/false, packed, kind, ast);
+    }]>
+  ];
+
   let hasCustomAssemblyFormat = 1;
 
   let extraClassDeclaration = [{

diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -174,12 +174,11 @@ class CIRGenBuilderTy : public mlir::OpBuilder {
       isZero &= isNullValue(typedAttr);
     }
 
-    // Struct type not specified: create type from members.
+    // Struct type not specified: create anon struct type from members.
     if (!structTy)
-      structTy = getType<mlir::cir::StructType>(
-          members, mlir::StringAttr::get(getContext()),
-          /*incomplete=*/false, packed, mlir::cir::StructType::Struct,
-          /*ast=*/nullptr);
+      structTy = getType<mlir::cir::StructType>(members, packed,
+                                                mlir::cir::StructType::Struct,
+                                                /*ast=*/nullptr);
 
     // Return zero or anonymous constant struct.
     if (isZero)
@@ -199,7 +198,7 @@ class CIRGenBuilderTy : public mlir::OpBuilder {
     }
 
     if (!ty)
-      ty = getAnonStructTy(members, /*incomplete=*/false, packed);
+      ty = getAnonStructTy(members, packed);
 
     auto sTy = ty.dyn_cast<mlir::cir::StructType>();
     assert(sTy && "expected struct type");
@@ -396,9 +395,15 @@ class CIRGenBuilderTy : public mlir::OpBuilder {
 
   /// Get a CIR anonymous struct type.
   mlir::cir::StructType
-  getAnonStructTy(llvm::ArrayRef<mlir::Type> members, bool incomplete,
-                  bool packed = false, const clang::RecordDecl *ast = nullptr) {
-    return getStructTy(members, "", incomplete, packed, ast);
+  getAnonStructTy(llvm::ArrayRef<mlir::Type> members, bool packed = false,
+                  const clang::RecordDecl *ast = nullptr) {
+    mlir::cir::ASTRecordDeclAttr astAttr = nullptr;
+    auto kind = mlir::cir::StructType::RecordKind::Struct;
+    if (ast) {
+      astAttr = getAttr<mlir::cir::ASTRecordDeclAttr>(ast);
+      kind = getRecordKind(ast->getTagKind());
+    }
+    return getType<mlir::cir::StructType>(members, packed, kind, astAttr);
   }
 
   /// Get a CIR record kind from a AST declaration tag.
@@ -418,19 +423,29 @@ class CIRGenBuilderTy : public mlir::OpBuilder {
     }
   }
 
+  /// Get a incomplete CIR struct type.
+  mlir::cir::StructType getIncompleteStructTy(llvm::StringRef name,
+                                              const clang::RecordDecl *ast) {
+    const auto nameAttr = getStringAttr(name);
+    auto kind = mlir::cir::StructType::RecordKind::Struct;
+    if (ast)
+      kind = getRecordKind(ast->getTagKind());
+    return getType<mlir::cir::StructType>(nameAttr, kind);
+  }
+
   /// Get a CIR named struct type.
-  mlir::cir::StructType getStructTy(llvm::ArrayRef<mlir::Type> members,
-                                    llvm::StringRef name, bool incomplete,
-                                    bool packed, const clang::RecordDecl *ast) {
+  mlir::cir::StructType getCompleteStructTy(llvm::ArrayRef<mlir::Type> members,
+                                            llvm::StringRef name, bool packed,
+                                            const clang::RecordDecl *ast) {
     const auto nameAttr = getStringAttr(name);
     mlir::cir::ASTRecordDeclAttr astAttr = nullptr;
     auto kind = mlir::cir::StructType::RecordKind::Struct;
     if (ast) {
       astAttr = getAttr<mlir::cir::ASTRecordDeclAttr>(ast);
       kind = getRecordKind(ast->getTagKind());
     }
-    return mlir::cir::StructType::get(getContext(), members, nameAttr,
-                                      incomplete, packed, kind, astAttr);
+    return getType<mlir::cir::StructType>(members, nameAttr, packed, kind,
+                                          astAttr);
   }
 
   //

diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -167,8 +167,7 @@ mlir::Type CIRGenTypes::convertRecordDeclType(const clang::RecordDecl *RD) {
   // Handle forward decl / incomplete types.
   if (!entry) {
     auto name = getRecordTypeName(RD, "");
-    entry = Builder.getStructTy({}, name, /*incomplete=*/true, /*packed=*/false,
-                                RD);
+    entry = Builder.getIncompleteStructTy(name, RD);
     recordDeclTypes[key] = entry;
   }
 

diff --git a/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp b/clang/lib/CIR/CodeGen/CIRRecordLayoutBuilder.cpp
@@ -608,8 +608,9 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *D,
         builder.astRecordLayout.getSize()) {
       CIRRecordLowering baseBuilder(*this, D, /*Packed=*/builder.isPacked);
       auto baseIdentifier = getRecordTypeName(D, ".base");
-      *BaseTy = Builder.getStructTy(baseBuilder.fieldTypes, baseIdentifier,
-                                    /*incomplete=*/false, /*packed=*/false, D);
+      *BaseTy =
+          Builder.getCompleteStructTy(baseBuilder.fieldTypes, baseIdentifier,
+                                      /*packed=*/false, D);
       // TODO(cir): add something like addRecordTypeName
 
       // BaseTy and Ty must agree on their packedness for getCIRFieldNo to work
@@ -622,8 +623,9 @@ CIRGenTypes::computeRecordLayout(const RecordDecl *D,
   // Fill in the struct *after* computing the base type.  Filling in the body
   // signifies that the type is no longer opaque and record layout is complete,
   // but we may need to recursively layout D while laying D out as a base type.
-  *Ty = Builder.getStructTy(builder.fieldTypes, getRecordTypeName(D, ""),
-                            /*incomplete=*/false, /*packed=*/false, D);
+  *Ty =
+      Builder.getCompleteStructTy(builder.fieldTypes, getRecordTypeName(D, ""),
+                                  /*packed=*/false, D);
 
   auto RL = std::make_unique<CIRGenRecordLayout>(
       Ty ? *Ty : mlir::cir::StructType{},

diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp
@@ -108,6 +108,7 @@ Type StructType::parse(mlir::AsmParser &parser) {
   const auto loc = parser.getCurrentLocation();
   bool packed = false;
   RecordKind kind;
+  auto *context = parser.getContext();
 
   if (parser.parseLess())
     return {};
@@ -152,8 +153,20 @@ Type StructType::parse(mlir::AsmParser &parser) {
   if (parser.parseGreater())
     return {};
 
-  return StructType::get(parser.getContext(), members, name, incomplete, packed,
-                         kind, nullptr);
+  // Try to create the proper type.
+  mlir::Type type = {};
+  if (name && incomplete) { // Identified & incomplete
+    type = StructType::get(context, name, kind);
+  } else if (name && !incomplete) { // Identified & complete
+    type = StructType::get(context, members, name, packed, kind);
+  } else if (!name && !incomplete) { // anonymous
+    type = StructType::get(context, members, packed, kind);
+  } else {
+    parser.emitError(loc, "anonymous structs must be complete");
+    return {};
+  }
+
+  return type;
 }
 
 void StructType::print(mlir::AsmPrinter &printer) const {

diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c
@@ -22,7 +22,7 @@ void baz(void) {
   struct Foo f;
 }
 
-// CHECK-DAG: !ty_22Node22 = !cir.struct<struct "Node" incomplete #cir.record.decl.ast>
+// CHECK-DAG: !ty_22Node22 = !cir.struct<struct "Node" incomplete>
 // CHECK-DAG: !ty_22Node221 = !cir.struct<struct "Node" {!cir.ptr<!ty_22Node22>} #cir.record.decl.ast>
 // CHECK-DAG: !ty_22Bar22 = !cir.struct<struct "Bar" {!s32i, !s8i}>
 // CHECK-DAG: !ty_22Foo22 = !cir.struct<struct "Foo" {!s32i, !s8i, !ty_22Bar22}>

diff --git a/clang/test/CIR/CodeGen/vtable-rtti.cpp b/clang/test/CIR/CodeGen/vtable-rtti.cpp
@@ -17,12 +17,6 @@ class B : public A
     virtual ~B() noexcept {}
 };
 
-// Type info B.
-// CHECK: ![[TypeInfoB:ty_.*]] = !cir.struct<struct "" {!cir.ptr<!u8i>, !cir.ptr<!u8i>, !cir.ptr<!u8i>}>
-
-// vtable for A type
-// CHECK: ![[VTableTypeA:ty_.*]] = !cir.struct<struct "" {!cir.array<!cir.ptr<!u8i> x 5>}>
-
 // Class A
 // CHECK: ![[ClassA:ty_.*]] = !cir.struct<class "A" {!cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>} #cir.record.decl.ast>
 
@@ -57,7 +51,7 @@ class B : public A
 // CHECK:   }
 
 // Vtable definition for A
-// cir.global "private" external @_ZTV1A : ![[VTableTypeA]] {alignment = 8 : i64}
+// CHECK: cir.global "private" external @_ZTV1A : !cir.struct<struct {!cir.array<!cir.ptr<!u8i> x 5>}> {alignment = 8 : i64}
 
 // A ctor => @A::A()
 // Calls @A::A() and initialize __vptr with address of A's vtable
@@ -73,7 +67,7 @@ class B : public A
 // CHECK:  }
 
 // vtable for B
-// CHECK:   cir.global linkonce_odr @_ZTV1B = #cir.vtable<{#cir.const_array<[#cir.ptr<null> : !cir.ptr<!u8i>, #cir.global_view<@_ZTI1B> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1BD2Ev> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1BD0Ev> : !cir.ptr<!u8i>, #cir.global_view<@_ZNK1A5quackEv> : !cir.ptr<!u8i>]> : !cir.array<!cir.ptr<!u8i> x 5>}> : ![[VTableTypeA]]
+// CHECK:   cir.global linkonce_odr @_ZTV1B = #cir.vtable<{#cir.const_array<[#cir.ptr<null> : !cir.ptr<!u8i>, #cir.global_view<@_ZTI1B> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1BD2Ev> : !cir.ptr<!u8i>, #cir.global_view<@_ZN1BD0Ev> : !cir.ptr<!u8i>, #cir.global_view<@_ZNK1A5quackEv> : !cir.ptr<!u8i>]> : !cir.array<!cir.ptr<!u8i> x 5>}> : !cir.struct<struct {!cir.array<!cir.ptr<!u8i> x 5>}>
 
 // vtable for __cxxabiv1::__si_class_type_info
 // CHECK:   cir.global "private" external @_ZTVN10__cxxabiv120__si_class_type_infoE : !cir.ptr<!cir.ptr<!u8i>>
@@ -85,7 +79,7 @@ class B : public A
 // CHECK:   cir.global "private" constant external @_ZTI1A : !cir.ptr<!u8i>
 
 // typeinfo for B
-// CHECK: cir.global constant external @_ZTI1B = #cir.typeinfo<{#cir.global_view<@_ZTVN10__cxxabiv120__si_class_type_infoE, [#cir.int<2> : !s64i]> : !cir.ptr<!u8i>, #cir.global_view<@_ZTS1B> : !cir.ptr<!u8i>, #cir.global_view<@_ZTI1A> : !cir.ptr<!u8i>}> : ![[TypeInfoB]]
+// CHECK: cir.global constant external @_ZTI1B = #cir.typeinfo<{#cir.global_view<@_ZTVN10__cxxabiv120__si_class_type_infoE, [#cir.int<2> : !s64i]> : !cir.ptr<!u8i>, #cir.global_view<@_ZTS1B> : !cir.ptr<!u8i>, #cir.global_view<@_ZTI1A> : !cir.ptr<!u8i>}> : !cir.struct<struct {!cir.ptr<!u8i>, !cir.ptr<!u8i>, !cir.ptr<!u8i>}>
 
 // Checks for dtors in dtors.cpp
 

diff --git a/clang/test/CIR/IR/invalid.cir b/clang/test/CIR/IR/invalid.cir
@@ -548,3 +548,10 @@ module {
     cir.return
   }
 }
+
+
+// -----
+
+!u16i = !cir.int<u, 16>
+// expected-error@+1 {{anonymous structs must be complete}}
+!struct = !cir.struct<struct incomplete>