Skip to content

[CIR] Defer declarations and tentative definitions #141700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2025

Conversation

andykaylor
Copy link
Contributor

This change adds code to defer emitting declarations and tentative definitions until they are referenced or trigger by a call to CompleteTentativeDefinition. This is needed to avoid premature handling of declarations and definitions that might not be referenced in the current translation unit. It also avoids incorrectly adding an initializer to external declarations.

This change also updates the way the insertion location for globals is chosen so that all globals will be emitted together at the top of the module. This makes no functional difference, but it is very useful for writing sensible tests.

Some tests are modified in this change to reorder global variables so that they can be checked in the order in which they will be emitted.

This change adds code to defer emitting declarations and tentative
definitions until they are referenced or trigger by a call to
CompleteTentativeDefinition. This is needed to avoid premature handling
of declarations and definitions that might not be referenced in the
current translation unit. It also avoids incorrectly adding an
initializer to external declarations.

This change also updates the way the insertion location for globals is
chosen so that all globals will be emitted together at the top of the
module. This makes no functional difference, but it is very useful for
writing sensible tests.

Some tests are modified in this change to reorder global variables so
that they can be checked in the order in which they will be emitted.
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels May 28, 2025
@llvmbot
Copy link
Member

llvmbot commented May 28, 2025

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Andy Kaylor (andykaylor)

Changes

This change adds code to defer emitting declarations and tentative definitions until they are referenced or trigger by a call to CompleteTentativeDefinition. This is needed to avoid premature handling of declarations and definitions that might not be referenced in the current translation unit. It also avoids incorrectly adding an initializer to external declarations.

This change also updates the way the insertion location for globals is chosen so that all globals will be emitted together at the top of the module. This makes no functional difference, but it is very useful for writing sensible tests.

Some tests are modified in this change to reorder global variables so that they can be checked in the order in which they will be emitted.


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

13 Files Affected:

  • (modified) clang/include/clang/CIR/CIRGenerator.h (+2)
  • (modified) clang/include/clang/CIR/MissingFeatures.h (+1)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+46-15)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+4)
  • (modified) clang/lib/CIR/CodeGen/CIRGenerator.cpp (+7)
  • (modified) clang/lib/CIR/FrontendAction/CIRGenAction.cpp (+4)
  • (modified) clang/test/CIR/CodeGen/array.cpp (+16-10)
  • (modified) clang/test/CIR/CodeGen/basic.c (+23-24)
  • (modified) clang/test/CIR/CodeGen/basic.cpp (+26-27)
  • (modified) clang/test/CIR/CodeGen/string-literals.c (+4-3)
  • (modified) clang/test/CIR/CodeGen/struct.c (+6-6)
  • (modified) clang/test/CIR/Lowering/array.cpp (+12-6)
  • (modified) clang/test/CIR/Lowering/hello.c (+4-4)
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index 883dce9deb8e3..bb20fdf72693b 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -54,6 +54,8 @@ class CIRGenerator : public clang::ASTConsumer {
   ~CIRGenerator() override;
   void Initialize(clang::ASTContext &astContext) override;
   bool HandleTopLevelDecl(clang::DeclGroupRef group) override;
+  void CompleteTentativeDefinition(clang::VarDecl *d) override;
+
   mlir::ModuleOp getModule() const;
   mlir::MLIRContext &getMLIRContext() { return *mlirContext; };
   const mlir::MLIRContext &getMLIRContext() const { return *mlirContext; };
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index fb205e9cd85d1..56bf9b1130f12 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -201,6 +201,7 @@ struct MissingFeatures {
   static bool writebacks() { return false; }
   static bool cleanupsToDeactivate() { return false; }
   static bool stackBase() { return false; }
+  static bool deferredDecls() { return false; }
 
   // Missing types
   static bool dataMemberType() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 60a3048f548e8..a1c2bc2e2ee72 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -231,8 +231,20 @@ void CIRGenModule::emitGlobal(clang::GlobalDecl gd) {
       return;
     }
   } else {
-    assert(cast<VarDecl>(global)->isFileVarDecl() &&
-           "Cannot emit local var decl as global");
+    const auto *vd = cast<VarDecl>(global);
+    assert(vd->isFileVarDecl() && "Cannot emit local var decl as global.");
+    if (vd->isThisDeclarationADefinition() != VarDecl::Definition &&
+        !astContext.isMSStaticDataMemberInlineDefinition(vd)) {
+      assert(!cir::MissingFeatures::openMP());
+      // If this declaration may have caused an inline variable definition to
+      // change linkage, make sure that it's emitted.
+      if (astContext.getInlineVariableDefinitionKind(vd) ==
+          ASTContext::InlineVariableDefinitionKind::Strong)
+        getAddrOfGlobalVar(vd);
+      // Otherwise, we can ignore this declaration. The variable will be emitted
+      // on its first use.
+      return;
+    }
   }
 
   // TODO(CIR): Defer emitting some global definitions until later
@@ -279,22 +291,23 @@ cir::GlobalOp CIRGenModule::createGlobalOp(CIRGenModule &cgm,
   {
     mlir::OpBuilder::InsertionGuard guard(builder);
 
-    // Some global emissions are triggered while emitting a function, e.g.
-    // void s() { const char *s = "yolo"; ... }
-    //
-    // Be sure to insert global before the current function
-    CIRGenFunction *curCGF = cgm.curCGF;
-    if (curCGF)
-      builder.setInsertionPoint(curCGF->curFn);
-
-    g = builder.create<cir::GlobalOp>(loc, name, t);
-    if (!curCGF) {
-      if (insertPoint)
-        cgm.getModule().insert(insertPoint, g);
+    // If an insertion point is provided, we're replacing an existing global,
+    // otherwise, create the new global immediately after the last gloabl we
+    // emitted.
+    if (insertPoint) {
+      builder.setInsertionPoint(insertPoint);
+    } else {
+      // Group global operations together at the top of the module.
+      if (cgm.lastGlobalOp)
+        builder.setInsertionPointAfter(cgm.lastGlobalOp);
       else
-        cgm.getModule().push_back(g);
+        builder.setInsertionPointToStart(cgm.getModule().getBody());
     }
 
+    g = builder.create<cir::GlobalOp>(loc, name, t);
+    if (!insertPoint)
+      cgm.lastGlobalOp = g;
+
     // Default to private until we can judge based on the initializer,
     // since MLIR doesn't allow public declarations.
     mlir::SymbolTable::setSymbolVisibility(
@@ -1044,6 +1057,24 @@ StringRef CIRGenModule::getMangledName(GlobalDecl gd) {
   return mangledDeclNames[canonicalGd] = result.first->first();
 }
 
+void CIRGenModule::emitTentativeDefinition(const VarDecl *d) {
+  assert(!d->getInit() && "Cannot emit definite definitions here!");
+
+  StringRef mangledName = getMangledName(d);
+  mlir::Operation *gv = getGlobalValue(mangledName);
+
+  // If we already have a definition, not declaration, with the same mangled
+  // name, emitting of declaration is not required (and would actually overwrite
+  // the emitted definition).
+  if (gv && !cast<cir::GlobalOp>(gv).isDeclaration())
+    return;
+
+  assert(!cir::MissingFeatures::deferredDecls());
+
+  // The tentative definition is the only definition.
+  emitGlobalVarDefinition(d);
+}
+
 cir::FuncOp CIRGenModule::getOrCreateCIRFunction(
     StringRef mangledName, mlir::Type funcType, GlobalDecl gd, bool forVTable,
     bool dontDefer, bool isThunk, ForDefinition_t isForDefinition,
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index add1b40ab0aea..aac8fe72b1890 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -111,6 +111,8 @@ class CIRGenModule : public CIRGenTypeCache {
   /// Handling globals
   /// -------
 
+  mlir::Operation *lastGlobalOp = nullptr;
+
   mlir::Operation *getGlobalValue(llvm::StringRef ref);
 
   /// If the specified mangled name is not in the module, create and return an
@@ -194,6 +196,8 @@ class CIRGenModule : public CIRGenTypeCache {
 
   llvm::StringRef getMangledName(clang::GlobalDecl gd);
 
+  void emitTentativeDefinition(const VarDecl *d);
+
   static void setInitializer(cir::GlobalOp &op, mlir::Attribute value);
 
   cir::FuncOp
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index 40252ffecfba1..726da5b013264 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -62,3 +62,10 @@ bool CIRGenerator::HandleTopLevelDecl(DeclGroupRef group) {
 
   return true;
 }
+
+void CIRGenerator::CompleteTentativeDefinition(VarDecl *d) {
+  if (diags.hasErrorOccurred())
+    return;
+
+  cgm->emitTentativeDefinition(d);
+}
diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
index cc65c93f5f16b..48ae2a05df700 100644
--- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
+++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
@@ -135,6 +135,10 @@ class CIRGenConsumer : public clang::ASTConsumer {
     }
     }
   }
+
+  void CompleteTentativeDefinition(VarDecl *D) override {
+    Gen->CompleteTentativeDefinition(D);
+  }
 };
 } // namespace cir
 
diff --git a/clang/test/CIR/CodeGen/array.cpp b/clang/test/CIR/CodeGen/array.cpp
index a4b9398df3b1a..18f30b20ff8f7 100644
--- a/clang/test/CIR/CodeGen/array.cpp
+++ b/clang/test/CIR/CodeGen/array.cpp
@@ -19,16 +19,6 @@ int aa[10][5];
 
 // OGCG: @aa = global [10 x [5 x i32]] zeroinitializer
 
-extern int b[10];
-// CIR: cir.global external @b = #cir.zero : !cir.array<!s32i x 10>
-
-// LLVM: @b = dso_local global [10 x i32] zeroinitializer
-
-extern int bb[10][5];
-// CIR: cir.global external @bb = #cir.zero : !cir.array<!cir.array<!s32i x 5> x 10>
-
-// LLVM: @bb = dso_local global [10 x [5 x i32]] zeroinitializer
-
 int c[10] = {};
 // CIR: cir.global external @c = #cir.zero : !cir.array<!s32i x 10>
 
@@ -66,6 +56,22 @@ int f[5] = {1, 2};
 
 // OGCG: @f = global [5 x i32] [i32 1, i32 2, i32 0, i32 0, i32 0]
 
+extern int b[10];
+// CIR: cir.global external @b : !cir.array<!s32i x 10>
+// LLVM: @b = external dso_local global [10 x i32]
+// OGCG: @b = external global [10 x i32]
+
+extern int bb[10][5];
+// CIR: cir.global external @bb : !cir.array<!cir.array<!s32i x 5> x 10>
+// LLVM: @bb = external dso_local global [10 x [5 x i32]]
+// OGCG: @bb = external global [10 x [5 x i32]]
+
+// This function is only here to make sure the external globals are emitted.
+void reference_externs() {
+  b;
+  bb;
+}
+
 // OGCG: @[[FUN2_ARR:.*]] = private unnamed_addr constant [2 x i32] [i32 5, i32 0], align 4
 // OGCG: @[[FUN3_ARR:.*]] = private unnamed_addr constant [2 x i32] [i32 5, i32 6], align 4
 // OGCG: @[[FUN4_ARR:.*]] = private unnamed_addr constant [2 x [1 x i32]] [
diff --git a/clang/test/CIR/CodeGen/basic.c b/clang/test/CIR/CodeGen/basic.c
index 835885cb3f4d1..abc1a45fd433f 100644
--- a/clang/test/CIR/CodeGen/basic.c
+++ b/clang/test/CIR/CodeGen/basic.c
@@ -5,6 +5,28 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll
 // RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG
 
+enum A {
+  A_one,
+  A_two
+};
+enum A a;
+
+// CHECK:   cir.global external @a = #cir.int<0> : !u32i
+
+enum B : int;
+enum B b;
+
+// CHECK:   cir.global external @b = #cir.int<0> : !u32i
+
+
+enum C : int {
+  C_one,
+  C_two
+};
+enum C c;
+
+// CHECK:   cir.global external @c = #cir.int<0> : !u32i
+
 int f1(int i);
 
 int f1(int i) {
@@ -12,8 +34,7 @@ int f1(int i) {
   return i;
 }
 
-//      CIR: module
-// CIR-NEXT: cir.func @f1(%arg0: !s32i loc({{.*}})) -> !s32i
+// CIR:      cir.func @f1(%arg0: !s32i loc({{.*}})) -> !s32i
 // CIR-NEXT:   %[[I_PTR:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i", init] {alignment = 4 : i64}
 // CIR-NEXT:   %[[RV:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
 // CIR-NEXT:   cir.store{{.*}} %arg0, %[[I_PTR]] : !s32i, !cir.ptr<!s32i>
@@ -288,25 +309,3 @@ size_type max_size(void) {
 // CHECK:   %6 = cir.load{{.*}} %0 : !cir.ptr<!u64i>, !u64i
 // CHECK:   cir.return %6 : !u64i
 // CHECK:   }
-
-enum A {
-  A_one,
-  A_two
-};
-enum A a;
-
-// CHECK:   cir.global external @a = #cir.int<0> : !u32i
-
-enum B : int;
-enum B b;
-
-// CHECK:   cir.global external @b = #cir.int<0> : !u32i
-
-
-enum C : int {
-  C_one,
-  C_two
-};
-enum C c;
-
-// CHECK:   cir.global external @c = #cir.int<0> : !u32i
diff --git a/clang/test/CIR/CodeGen/basic.cpp b/clang/test/CIR/CodeGen/basic.cpp
index 53f13f13853fa..ed1c6d364a0ef 100644
--- a/clang/test/CIR/CodeGen/basic.cpp
+++ b/clang/test/CIR/CodeGen/basic.cpp
@@ -1,11 +1,36 @@
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - 2>&1 | FileCheck %s
 
+enum A {
+  A_one,
+  A_two
+};
+enum A a;
+
+// CHECK:   cir.global external @a = #cir.int<0> : !u32i
+
+enum B : int;
+enum B b;
+
+// CHECK:   cir.global external @b = #cir.int<0> : !s32i
+
+enum C : int {
+  C_one,
+  C_two
+};
+enum C c;
+
+// CHECK:   cir.global external @c = #cir.int<0> : !s32i
+
+enum class D : int;
+enum D d;
+
+// CHECK:   cir.global external @d = #cir.int<0> : !s32i
+
 int f1() {
   int i;
   return i;
 }
 
-// CHECK: module
 // CHECK: cir.func @_Z2f1v() -> !s32i
 // CHECK:    %[[RV:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
 // CHECK:    %[[I_PTR:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64}
@@ -145,29 +170,3 @@ void ref_local(short x) {
 // CHECK:   %[[Y_REF_ADDR:.*]] = cir.alloca !cir.ptr<!s16i>, !cir.ptr<!cir.ptr<!s16i>>, ["y", init, const] {alignment = 8 : i64}
 // CHECK:   cir.store{{.*}} %[[ARG]], %[[X_ADDR]] : !s16i, !cir.ptr<!s16i>
 // CHECK:   cir.store{{.*}} %[[X_ADDR]], %[[Y_REF_ADDR]] : !cir.ptr<!s16i>, !cir.ptr<!cir.ptr<!s16i>>
-
-enum A {
-  A_one,
-  A_two
-};
-enum A a;
-
-// CHECK:   cir.global external @a = #cir.int<0> : !u32i
-
-enum B : int;
-enum B b;
-
-// CHECK:   cir.global external @b = #cir.int<0> : !s32i
-
-enum C : int {
-  C_one,
-  C_two
-};
-enum C c;
-
-// CHECK:   cir.global external @c = #cir.int<0> : !s32i
-
-enum class D : int;
-enum D d;
-
-// CHECK:   cir.global external @d = #cir.int<0> : !s32i
diff --git a/clang/test/CIR/CodeGen/string-literals.c b/clang/test/CIR/CodeGen/string-literals.c
index 873b00d9c9a98..81d2a27591295 100644
--- a/clang/test/CIR/CodeGen/string-literals.c
+++ b/clang/test/CIR/CodeGen/string-literals.c
@@ -5,6 +5,10 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-android21 -emit-llvm %s -o %t.ll
 // RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
 
+// CIR: cir.global external @[[STR1_GLOBAL:.*]] = #cir.const_array<"1\00" : !cir.array<!s8i x 2>> : !cir.array<!s8i x 2>
+// CIR: cir.global external @[[STR2_GLOBAL:.*]] = #cir.zero : !cir.array<!s8i x 1>
+// CIR: cir.global external @[[STR3_GLOBAL:.*]] = #cir.zero : !cir.array<!s8i x 2>
+
 // LLVM: @[[STR1_GLOBAL:.*]] = dso_local global [2 x i8] c"1\00"
 // LLVM: @[[STR2_GLOBAL:.*]] = dso_local global [1 x i8] zeroinitializer
 // LLVM: @[[STR3_GLOBAL:.*]] = dso_local global [2 x i8] zeroinitializer
@@ -17,7 +21,6 @@ char *f1() {
   return "1";
 }
 
-// CIR: cir.global external @[[STR1_GLOBAL:.*]] = #cir.const_array<"1\00" : !cir.array<!s8i x 2>> : !cir.array<!s8i x 2>
 // CIR: cir.func @f1()
 // CIR:   %[[STR:.*]] = cir.get_global @[[STR1_GLOBAL]] : !cir.ptr<!cir.array<!s8i x 2>>
 
@@ -31,7 +34,6 @@ char *f2() {
   return "";
 }
 
-// CIR: cir.global external @[[STR2_GLOBAL:.*]] = #cir.zero : !cir.array<!s8i x 1>
 // CIR: cir.func @f2()
 // CIR:   %[[STR2:.*]] = cir.get_global @[[STR2_GLOBAL]] : !cir.ptr<!cir.array<!s8i x 1>>
 
@@ -45,7 +47,6 @@ char *f3() {
   return "\00";
 }
 
-// CIR: cir.global external @[[STR3_GLOBAL:.*]] = #cir.zero : !cir.array<!s8i x 2>
 // CIR: cir.func @f3()
 // CIR:   %[[STR3:.*]] = cir.get_global @[[STR3_GLOBAL]] : !cir.ptr<!cir.array<!s8i x 2>>
 
diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c
index 9c4a730d19a83..4c18de295c68c 100644
--- a/clang/test/CIR/CodeGen/struct.c
+++ b/clang/test/CIR/CodeGen/struct.c
@@ -42,12 +42,6 @@
 // OGCG-DAG: %struct.CycleMiddle = type { ptr }
 // OGCG-DAG: %struct.CycleEnd = type { ptr }
 
-struct IncompleteS *p;
-
-// CIR:      cir.global external @p = #cir.ptr<null> : !cir.ptr<!rec_IncompleteS>
-// LLVM-DAG: @p = dso_local global ptr null
-// OGCG-DAG: @p = global ptr null, align 8
-
 struct CompleteS {
   int a;
   char b;
@@ -57,6 +51,12 @@ struct CompleteS {
 // LLVM-DAG:  @cs = dso_local global %struct.CompleteS zeroinitializer
 // OGCG-DAG:  @cs = global %struct.CompleteS zeroinitializer, align 4
 
+struct IncompleteS *p;
+
+// CIR:      cir.global external @p = #cir.ptr<null> : !cir.ptr<!rec_IncompleteS>
+// LLVM-DAG: @p = dso_local global ptr null
+// OGCG-DAG: @p = global ptr null, align 8
+
 struct InnerS {
   int a;
   char b;
diff --git a/clang/test/CIR/Lowering/array.cpp b/clang/test/CIR/Lowering/array.cpp
index 4d9161d77663f..335042ca68f4c 100644
--- a/clang/test/CIR/Lowering/array.cpp
+++ b/clang/test/CIR/Lowering/array.cpp
@@ -7,12 +7,6 @@ int a[10];
 int aa[10][5];
 // CHECK: @aa = dso_local global [10 x [5 x i32]] zeroinitializer
 
-extern int b[10];
-// CHECK: @b = dso_local global [10 x i32] zeroinitializer
-
-extern int bb[10][5];
-// CHECK: @bb = dso_local global [10 x [5 x i32]] zeroinitializer
-
 int c[10] = {};
 // CHECK: @c = dso_local global [10 x i32] zeroinitializer
 
@@ -30,6 +24,18 @@ int e[10] = {1, 2};
 int f[5] = {1, 2};
 // CHECK: @f = dso_local global [5 x i32] [i32 1, i32 2, i32 0, i32 0, i32 0]
 
+extern int b[10];
+// CHECK: @b = external dso_local global [10 x i32]
+
+extern int bb[10][5];
+// CHECK: @bb = external dso_local global [10 x [5 x i32]]
+
+// This function is only here to make sure the external globals are emitted.
+void reference_externs() {
+  b;
+  bb;
+}
+
 void func() {
   int arr[10];
   int e = arr[0];
diff --git a/clang/test/CIR/Lowering/hello.c b/clang/test/CIR/Lowering/hello.c
index f45beafdcb533..21adf17b73c09 100644
--- a/clang/test/CIR/Lowering/hello.c
+++ b/clang/test/CIR/Lowering/hello.c
@@ -1,10 +1,10 @@
 // Smoke test for ClangIR-to-LLVM IR code generation
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o -  | FileCheck %s
 
-int a;
-
-// CHECK: @a = dso_local global i32 0
-
 int b = 2;
 
 // CHECK: @b = dso_local global i32 2
+
+int a;
+
+// CHECK: @a = dso_local global i32 0

Copy link
Contributor

@xlauko xlauko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, with minor nits

Comment on lines +139 to +141
void CompleteTentativeDefinition(VarDecl *D) override {
Gen->CompleteTentativeDefinition(D);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void CompleteTentativeDefinition(VarDecl *D) override {
Gen->CompleteTentativeDefinition(D);
}
void CompleteTentativeDefinition(VarDecl *d) override {
Gen->CompleteTentativeDefinition(d);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using clang naming style in this file based on previous agreement with the clang maintainers.

// If we already have a definition, not declaration, with the same mangled
// name, emitting of declaration is not required (and would actually overwrite
// the emitted definition).
if (gv && !cast<cir::GlobalOp>(gv).isDeclaration())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (gv && !cast<cir::GlobalOp>(gv).isDeclaration())
if (gv && !mlir::cast<cir::GlobalOp>(gv).isDeclaration())

@andykaylor andykaylor merged commit b574c81 into llvm:main May 28, 2025
11 checks passed
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
This change adds code to defer emitting declarations and tentative
definitions until they are referenced or trigger by a call to
CompleteTentativeDefinition. This is needed to avoid premature handling
of declarations and definitions that might not be referenced in the
current translation unit. It also avoids incorrectly adding an
initializer to external declarations.

This change also updates the way the insertion location for globals is
chosen so that all globals will be emitted together at the top of the
module. This makes no functional difference, but it is very useful for
writing sensible tests.

Some tests are modified in this change to reorder global variables so
that they can be checked in the order in which they will be emitted.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
This change adds code to defer emitting declarations and tentative
definitions until they are referenced or trigger by a call to
CompleteTentativeDefinition. This is needed to avoid premature handling
of declarations and definitions that might not be referenced in the
current translation unit. It also avoids incorrectly adding an
initializer to external declarations.

This change also updates the way the insertion location for globals is
chosen so that all globals will be emitted together at the top of the
module. This makes no functional difference, but it is very useful for
writing sensible tests.

Some tests are modified in this change to reorder global variables so
that they can be checked in the order in which they will be emitted.
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.

4 participants