Skip to content

Conversation

andykaylor
Copy link
Contributor

This adds the needed handling for completing record types which were previously declared leading us to create an incomplete record type.

This adds the needed handling for completing record types which were
previously declared leading us to create an incomplete record type.
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jun 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2025

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This adds the needed handling for completing record types which were previously declared leading us to create an incomplete record type.


Full diff: https://github.com/llvm/llvm-project/pull/143176.diff

10 Files Affected:

  • (modified) clang/include/clang/CIR/CIRGenerator.h (+1)
  • (modified) clang/include/clang/CIR/MissingFeatures.h (-1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+6)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+3)
  • (modified) clang/lib/CIR/CodeGen/CIRGenTypes.cpp (+37-2)
  • (modified) clang/lib/CIR/CodeGen/CIRGenTypes.h (+4)
  • (modified) clang/lib/CIR/CodeGen/CIRGenerator.cpp (+23)
  • (modified) clang/lib/CIR/FrontendAction/CIRGenAction.cpp (+7)
  • (added) clang/test/CIR/CodeGen/forward-decls.cpp (+124)
  • (added) clang/test/CIR/CodeGen/forward-enum.c (+16)
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index 106c5a6f99c7c..dd48eec238fca 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -78,6 +78,7 @@ class CIRGenerator : public clang::ASTConsumer {
   bool HandleTopLevelDecl(clang::DeclGroupRef group) override;
   void HandleTranslationUnit(clang::ASTContext &astContext) override;
   void HandleInlineFunctionDefinition(clang::FunctionDecl *d) override;
+  void HandleTagDeclDefinition(clang::TagDecl *d) override;
   void CompleteTentativeDefinition(clang::VarDecl *d) override;
 
   mlir::ModuleOp getModule() const;
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index f1e0c15d41f64..7fcb6cb95db4a 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -199,7 +199,6 @@ struct MissingFeatures {
   static bool msabi() { return false; }
   static bool typeChecks() { return false; }
   static bool lambdaFieldToName() { return false; }
-  static bool updateCompletedType() { return false; }
   static bool moduleNameHash() { return false; }
   static bool constantFoldSwitchStatement() { return false; }
   static bool cudaSupport() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index ce3c57e5f20a8..e5eae31e937d3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -838,6 +838,11 @@ void CIRGenModule::maybeSetTrivialComdat(const Decl &d, mlir::Operation *op) {
   assert(!cir::MissingFeatures::opFuncSetComdat());
 }
 
+void CIRGenModule::updateCompletedType(const TagDecl *td) {
+  // Make sure that this type is translated.
+  genTypes.updateCompletedType(td);
+}
+
 // TODO(CIR): this could be a common method between LLVM codegen.
 static bool isVarDeclStrongDefinition(const ASTContext &astContext,
                                       CIRGenModule &cgm, const VarDecl *vd,
@@ -1145,6 +1150,7 @@ void CIRGenModule::emitTopLevelDecl(Decl *decl) {
 
   // No code generation needed.
   case Decl::UsingShadow:
+  case Decl::Empty:
     break;
 
   // C++ Decls
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 7055170ac7ac3..ac25d52472050 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -241,6 +241,9 @@ class CIRGenModule : public CIRGenTypeCache {
 
   void emitTentativeDefinition(const VarDecl *d);
 
+  // Make sure that this type is translated.
+  void updateCompletedType(const clang::TagDecl *td);
+
   bool supportsCOMDAT() const;
   void maybeSetTrivialComdat(const clang::Decl &d, mlir::Operation *op);
 
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index 5f51d348e75ce..eaba3dfd1105e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -432,8 +432,6 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
   }
 
   case Type::Enum: {
-    // TODO(cir): Implement updateCompletedType for enums.
-    assert(!cir::MissingFeatures::updateCompletedType());
     const EnumDecl *ED = cast<EnumType>(ty)->getDecl();
     if (auto integerType = ED->getIntegerType(); !integerType.isNull())
       return convertType(integerType);
@@ -586,3 +584,40 @@ const CIRGenFunctionInfo &CIRGenTypes::arrangeGlobalDeclaration(GlobalDecl gd) {
 
   return arrangeFunctionDeclaration(fd);
 }
+
+// When we find the full definition for a TagDecl, replace the 'opaque' type we
+// previously made for it if applicable.
+void CIRGenTypes::updateCompletedType(const TagDecl *td) {
+  // If this is an enum being completed, then we flush all non-struct types
+  // from the cache. This allows function types and other things that may be
+  // derived from the enum to be recomputed.
+  if (const auto *ed = dyn_cast<EnumDecl>(td)) {
+    // Classic codegen clears the type cache if it contains an entry for this
+    // enum type that doesn't use i32 as the underlying type, but I can't find
+    // a test case that meets that condition. C++ doesn't allow forward
+    // declaration of enums, and C doesn't allow an incomplete forward
+    // declaration with a non-default type.
+    assert(
+        !typeCache.count(ed->getTypeForDecl()) ||
+        (convertType(ed->getIntegerType()) == typeCache[ed->getTypeForDecl()]));
+    // If necessary, provide the full definition of a type only used with a
+    // declaration so far.
+    assert(!cir::MissingFeatures::generateDebugInfo());
+    return;
+  }
+
+  // If we completed a RecordDecl that we previously used and converted to an
+  // anonymous type, then go ahead and complete it now.
+  const auto *rd = cast<RecordDecl>(td);
+  if (rd->isDependentType())
+    return;
+
+  // Only complete if we converted it already. If we haven't converted it yet,
+  // we'll just do it lazily.
+  if (recordDeclTypes.count(astContext.getTagDeclType(rd).getTypePtr()))
+    convertRecordDeclType(rd);
+
+  // If necessary, provide the full definition of a type only used with a
+  // declaration so far.
+  assert(!cir::MissingFeatures::generateDebugInfo());
+}
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.h b/clang/lib/CIR/CodeGen/CIRGenTypes.h
index 7a4301ed38d04..48d474beeddec 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.h
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.h
@@ -151,6 +151,10 @@ class CIRGenTypes {
 
   const CIRGenFunctionInfo &arrangeGlobalDeclaration(GlobalDecl gd);
 
+  /// UpdateCompletedType - when we find the full definition for a TagDecl,
+  /// replace the 'opaque' type we previously made for it if applicable.
+  void updateCompletedType(const clang::TagDecl *td);
+
   /// Free functions are functions that are compatible with an ordinary C
   /// function pointer type.
   const CIRGenFunctionInfo &
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 0503b49298ce8..99d652841be27 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -129,6 +129,29 @@ void CIRGenerator::emitDeferredDecls() {
   deferredInlineMemberFuncDefs.clear();
 }
 
+/// HandleTagDeclDefinition - This callback is invoked each time a TagDecl to
+/// (e.g. struct, union, enum, class) is completed. This allows the client to
+/// hack on the type, which can occur at any point in the file (because these
+/// can be defined in declspecs).
+void CIRGenerator::HandleTagDeclDefinition(TagDecl *d) {
+  if (diags.hasErrorOccurred())
+    return;
+
+  // Don't allow re-entrant calls to CIRGen triggered by PCH deserialization to
+  // emit deferred decls.
+  HandlingTopLevelDeclRAII handlingDecl(*this, /*EmitDeferred=*/false);
+
+  cgm->updateCompletedType(d);
+
+  // For MSVC compatibility, treat declarations of static data members with
+  // inline initializers as definitions.
+  if (astContext->getTargetInfo().getCXXABI().isMicrosoft())
+    cgm->errorNYI(d->getSourceRange(), "HandleTagDeclDefinition: MSABI");
+  // For OpenMP emit declare reduction functions, if required.
+  if (astContext->getLangOpts().OpenMP)
+    cgm->errorNYI(d->getSourceRange(), "HandleTagDeclDefinition: OpenMP");
+}
+
 void CIRGenerator::CompleteTentativeDefinition(VarDecl *d) {
   if (diags.hasErrorOccurred())
     return;
diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
index c19312ae245dd..9264aa6b18b58 100644
--- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
+++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
@@ -140,6 +140,13 @@ class CIRGenConsumer : public clang::ASTConsumer {
     }
   }
 
+  void HandleTagDeclDefinition(TagDecl *D) override {
+    PrettyStackTraceDecl CrashInfo(D, SourceLocation(),
+                                   Context->getSourceManager(),
+                                   "CIR generation of declaration");
+    Gen->HandleTagDeclDefinition(D);
+  }
+
   void CompleteTentativeDefinition(VarDecl *D) override {
     Gen->CompleteTentativeDefinition(D);
   }
diff --git a/clang/test/CIR/CodeGen/forward-decls.cpp b/clang/test/CIR/CodeGen/forward-decls.cpp
new file mode 100644
index 0000000000000..5a7e6b0afbed2
--- /dev/null
+++ b/clang/test/CIR/CodeGen/forward-decls.cpp
@@ -0,0 +1,124 @@
+// RUN: split-file %s %t
+
+
+//--- incomplete_struct
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %t/incomplete_struct -o %t/incomplete_struct.cir
+// RUN: FileCheck %s --input-file=%t/incomplete_struct.cir --check-prefix=CHECK1
+
+// Forward declaration of the record is never defined, so it is created as
+// an incomplete struct in CIR and will remain as such.
+
+// CHECK1: ![[INC_STRUCT:.+]] = !cir.record<struct "IncompleteStruct" incomplete>
+struct IncompleteStruct;
+// CHECK1: testIncompleteStruct(%arg0: !cir.ptr<![[INC_STRUCT]]>
+void testIncompleteStruct(struct IncompleteStruct *s) {};
+
+
+
+//--- mutated_struct
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %t/mutated_struct -o %t/mutated_struct.cir
+// RUN: FileCheck %s --input-file=%t/mutated_struct.cir --check-prefix=CHECK2
+
+// Foward declaration of the struct is followed by usage, then definition.
+// This means it will initially be created as incomplete, then completed.
+
+// CHECK2: ![[COMPLETE:.+]] = !cir.record<struct "ForwardDeclaredStruct" {!s32i}>
+// CHECK2: testForwardDeclaredStruct(%arg0: !cir.ptr<![[COMPLETE]]>
+struct ForwardDeclaredStruct;
+void testForwardDeclaredStruct(struct ForwardDeclaredStruct *fds) {};
+struct ForwardDeclaredStruct {
+  int testVal;
+};
+
+
+
+//--- recursive_struct
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %t/recursive_struct -o %t/recursive_struct.cir
+// RUN: FileCheck --check-prefix=CHECK3 --input-file=%t/recursive_struct.cir %s
+
+// Struct is initially forward declared since the self-reference is generated
+// first. Then, once the type is fully generated, it is completed.
+
+// CHECK3: ![[STRUCT:.+]] = !cir.record<struct "RecursiveStruct" {!s32i, !cir.ptr<!cir.record<struct "RecursiveStruct">>}>
+struct RecursiveStruct {
+  int value;
+  struct RecursiveStruct *next;
+};
+// CHECK3: testRecursiveStruct(%arg0: !cir.ptr<![[STRUCT]]>
+void testRecursiveStruct(struct RecursiveStruct *arg) {
+  // CHECK3: %[[#NEXT:]] = cir.get_member %{{.+}}[1] {name = "next"} : !cir.ptr<![[STRUCT]]> -> !cir.ptr<!cir.ptr<![[STRUCT]]>>
+  // CHECK3: %[[#DEREF:]] = cir.load{{.*}} %[[#NEXT]] : !cir.ptr<!cir.ptr<![[STRUCT]]>>, !cir.ptr<![[STRUCT]]>
+  // CHECK3: cir.get_member %[[#DEREF]][0] {name = "value"} : !cir.ptr<![[STRUCT]]> -> !cir.ptr<!s32i>
+  arg->next->value;
+}
+
+
+
+//--- indirect_recursive_struct
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %t/indirect_recursive_struct -o %t/indirect_recursive_struct.cir
+// RUN: FileCheck --check-prefix=CHECK4 --input-file=%t/indirect_recursive_struct.cir %s
+
+// Node B refers to A, and vice-versa, so a forward declaration is used to
+// ensure the classes can be defined. Since types alias are not yet supported
+// in recursive type, each struct is expanded until there are no more recursive
+// types, or all the recursive types are self references.
+
+// CHECK4: ![[B:.+]] = !cir.record<struct "StructNodeB" {!s32i, !cir.ptr<!cir.record<struct "StructNodeA" {!s32i, !cir.ptr<!cir.record<struct "StructNodeB">>}
+// CHECK4: ![[A:.+]] = !cir.record<struct "StructNodeA" {!s32i, !cir.ptr<![[B]]>}>
+struct StructNodeB;
+struct StructNodeA {
+  int value;
+  struct StructNodeB *next;
+};
+struct StructNodeB {
+  int value;
+  struct StructNodeA *next;
+};
+
+void testIndirectSelfReference(struct StructNodeA arg) {
+  // CHECK4: %[[#V1:]] = cir.get_member %{{.+}}[1] {name = "next"} : !cir.ptr<![[A]]> -> !cir.ptr<!cir.ptr<![[B]]>>
+  // CHECK4: %[[#V2:]] = cir.load{{.*}} %[[#V1]] : !cir.ptr<!cir.ptr<![[B]]>>, !cir.ptr<![[B]]>
+  // CHECK4: %[[#V3:]] = cir.get_member %[[#V2]][1] {name = "next"} : !cir.ptr<![[B]]> -> !cir.ptr<!cir.ptr<![[A]]>>
+  // CHECK4: %[[#V4:]] = cir.load{{.*}} %[[#V3]] : !cir.ptr<!cir.ptr<![[A]]>>, !cir.ptr<![[A]]>
+  // CHECK4: cir.get_member %[[#V4]][0] {name = "value"} : !cir.ptr<![[A]]> -> !cir.ptr<!s32i>
+  arg.next->next->value;
+}
+
+
+
+//--- complex_struct
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %t/complex_struct -o %t/complex_struct.cir
+// RUN: FileCheck --check-prefix=CHECK5 --input-file=%t/complex_struct.cir %s
+
+// A sizeable complex struct just to double check that stuff is working.
+// CHECK5: !cir.record<struct "anon.0" {!cir.ptr<!cir.record<struct "A" {!cir.record<struct "anon.0">, !cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !cir.record<struct "C" {!cir.ptr<!cir.record<struct "A">>, !cir.ptr<!cir.record<struct "B">>, !cir.ptr<!cir.record<struct "C">>}>, !cir.record<union "anon.1" {!cir.ptr<!cir.record<struct "A">>, !cir.record<struct "anon.2" {!cir.ptr<!cir.record<struct "B">>}>}>}>}>>}>
+// CHECK5: !cir.record<struct "C" {!cir.ptr<!cir.record<struct "A" {!rec_anon2E0, !cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !cir.record<struct "C">, !cir.record<union "anon.1" {!cir.ptr<!cir.record<struct "A">>, !cir.record<struct "anon.2" {!cir.ptr<!cir.record<struct "B">>}>}>}>}>>, !cir.ptr<!cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !cir.record<struct "C">, !cir.record<union "anon.1" {!cir.ptr<!cir.record<struct "A" {!rec_anon2E0, !cir.record<struct "B">}>>, !cir.record<struct "anon.2" {!cir.ptr<!cir.record<struct "B">>}>}>}>>, !cir.ptr<!cir.record<struct "C">>}>
+// CHECK5: !cir.record<struct "anon.2" {!cir.ptr<!cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !rec_C, !cir.record<union "anon.1" {!cir.ptr<!cir.record<struct "A" {!rec_anon2E0, !cir.record<struct "B">}>>, !cir.record<struct "anon.2">}>}>>}>
+// CHECK5: !cir.record<union "anon.1" {!cir.ptr<!cir.record<struct "A" {!rec_anon2E0, !cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !rec_C, !cir.record<union "anon.1">}>}>>, !rec_anon2E2}>
+// CHECK5: !cir.record<struct "B" {!cir.ptr<!cir.record<struct "B">>, !rec_C, !rec_anon2E1}>
+// CHECK5: !cir.record<struct "A" {!rec_anon2E0, !rec_B}>
+struct A {
+  struct {
+    struct A *a1;
+  };
+  struct B {
+    struct B *b1;
+    struct C {
+      struct A *a2;
+      struct B *b2;
+      struct C *c1;
+    } c;
+    union {
+      struct A *a2;
+      struct {
+        struct B *b3;
+      };
+    } u;
+  } b;
+};
+void test(struct A *a){};
diff --git a/clang/test/CIR/CodeGen/forward-enum.c b/clang/test/CIR/CodeGen/forward-enum.c
new file mode 100644
index 0000000000000..f5479262908ac
--- /dev/null
+++ b/clang/test/CIR/CodeGen/forward-enum.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck %s --input-file=%t.cir
+
+extern enum X x;
+void f(void) {
+  x;
+}
+
+enum X {
+  One,
+  Two
+};
+
+// CHECK: cir.global "private" external @x : !u32i
+// CHECK: cir.func{{.*}} @f
+// CHECK:   cir.get_global @x : !cir.ptr<!u32i>

@andykaylor andykaylor merged commit 16dda4d into llvm:main Jun 6, 2025
10 checks passed
@andykaylor andykaylor deleted the cir-forward-decls branch June 6, 2025 19:56
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
This adds the needed handling for completing record types which were
previously declared leading us to create an incomplete record type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants