Skip to content

[CIR] Add emitDeclInvariant for global with constant storage #171915

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.

Sign up for GitHub

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

Jump to bottom
Open
adams381 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[CIR] Add emitDeclInvariant for global with constant storage #171915

adams381 wants to merge 3 commits into from
+330 −13

Conversation

@adams381
Copy link
Contributor

@adams381 adams381 commented Dec 11, 2025

Implement emitDeclInvariant to emit llvm.invariant.start intrinsic for global variables with constant storage. This enables optimizations by marking when a global becomes read-only after initialization.

Changes

  • Add emitDeclInvariant and emitInvariantStart functions in CIRGenCXX.cpp
  • Add emitInvariantStart declaration in CIRGenFunction.h
  • Update emitCXXGlobalVarDeclInit to call emitDeclInvariant for constant storage globals after initialization
  • Update getOrCreateCIRGlobal to set constant flag on globals with constant storage
  • Add comprehensive test covering positive and negative cases

Implementation Details

The implementation handles address spaces correctly, dynamically constructing the intrinsic name (e.g., invariant.start.p0, invariant.start.p10) based on the pointer's address space. The intrinsic is only emitted when optimizations are enabled, matching classic codegen behavior.

Testing

All tests pass (411/412, 1 unsupported). The test file includes CIR, LLVM, and OGCG checks for both optimized and non-optimized builds.

@adams381
[CIR] Add emitDeclInvariant for global with constant storage
517a1a2 
Implement emitDeclInvariant to emit llvm.invariant.start intrinsic for
global variables with constant storage. This enables optimizations by
marking when a global becomes read-only after initialization.

Changes:
- Add emitDeclInvariant and emitInvariantStart functions in CIRGenCXX.cpp
- Add emitInvariantStart declaration in CIRGenFunction.h
- Update emitCXXGlobalVarDeclInit to call emitDeclInvariant for constant
  storage globals after initialization
- Update getOrCreateCIRGlobal to set constant flag on globals with
  constant storage
- Add comprehensive test covering positive and negative cases

The implementation handles address spaces correctly, dynamically
constructing the intrinsic name (e.g., invariant.start.p0,
invariant.start.p10) based on the pointer's address space.
@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Dec 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2025

@llvm/pr-subscribers-clang

Author: None (adams381)

Changes

Implement emitDeclInvariant to emit llvm.invariant.start intrinsic for global variables with constant storage. This enables optimizations by marking when a global becomes read-only after initialization.

Changes

  • Add emitDeclInvariant and emitInvariantStart functions in CIRGenCXX.cpp
  • Add emitInvariantStart declaration in CIRGenFunction.h
  • Update emitCXXGlobalVarDeclInit to call emitDeclInvariant for constant storage globals after initialization
  • Update getOrCreateCIRGlobal to set constant flag on globals with constant storage
  • Add comprehensive test covering positive and negative cases

Implementation Details

The implementation handles address spaces correctly, dynamically constructing the intrinsic name (e.g., invariant.start.p0, invariant.start.p10) based on the pointer's address space. The intrinsic is only emitted when optimizations are enabled, matching classic codegen behavior.

Testing

All tests pass (411/412, 1 unsupported). The test file includes CIR, LLVM, and OGCG checks for both optimized and non-optimized builds.

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

4 Files Affected:

  • (modified) clang/lib/CIR/CodeGen/CIRGenCXX.cpp (+67-5)
  • (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+2)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+17-8)
  • (added) clang/test/CIR/CodeGen/global-constant-storage.cpp (+244)
diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
index 71568ec87a31b..c0890f6b42663 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
@@ -16,11 +16,54 @@
 
 #include "clang/AST/GlobalDecl.h"
 #include "clang/CIR/MissingFeatures.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace clang::CIRGen;
 
+/// Emit code to cause the variable at the given address to be considered as
+/// constant from this point onwards.
+static void emitDeclInvariant(CIRGenFunction &CGF, const VarDecl *D) {
+  mlir::Value addr = CGF.cgm.getAddrOfGlobalVar(D);
+  CGF.emitInvariantStart(CGF.getContext().getTypeSizeInChars(D->getType()),
+                         addr, CGF.getLoc(D->getSourceRange()));
+}
+
+void CIRGenFunction::emitInvariantStart(CharUnits Size, mlir::Value Addr,
+                                        mlir::Location loc) {
+  // Do not emit the intrinsic if we're not optimizing.
+  if (!cgm.getCodeGenOpts().OptimizationLevel)
+    return;
+
+  CIRGenBuilderTy &builder = getBuilder();
+
+  // Create the size constant as i64
+  uint64_t width = Size.getQuantity();
+  mlir::Value sizeValue = builder.getConstInt(loc, builder.getSInt64Ty(),
+                                              static_cast<int64_t>(width));
+
+  // Determine address space for intrinsic name
+  unsigned addrSpace = 0;
+  if (auto ptrTy = mlir::dyn_cast<cir::PointerType>(Addr.getType()))
+    addrSpace =
+        ptrTy.getAddrSpace() ? ptrTy.getAddrSpace().getValue().getUInt() : 0;
+
+  // Format intrinsic name with address space suffix (e.g.,
+  // "invariant.start.p0", "invariant.start.p10")
+  llvm::SmallString<32> intrinsicName;
+  llvm::raw_svector_ostream os(intrinsicName);
+  os << "invariant.start.p" << addrSpace;
+
+  // Create the intrinsic call. The llvm.invariant.start intrinsic returns a
+  // token, but we don't need to capture it. The return type is set to match
+  // the address type for consistency with the operation signature.
+  cir::LLVMIntrinsicCallOp::create(
+      builder, loc, builder.getStringAttr(intrinsicName), Addr.getType(),
+      mlir::ValueRange{sizeValue, Addr});
+}
+
 static void emitDeclInit(CIRGenFunction &cgf, const VarDecl *varDecl,
                          cir::GlobalOp globalOp) {
   assert((varDecl->hasGlobalStorage() ||
@@ -234,13 +277,32 @@ void CIRGenModule::emitCXXGlobalVarDeclInit(const VarDecl *varDecl,
 
     bool needsDtor = varDecl->needsDestruction(getASTContext()) ==
                      QualType::DK_cxx_destructor;
+    bool isConstantStorage =
+        varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor);
     // PerformInit, constant store invariant / destroy handled below.
-    if (performInit)
+    if (performInit) {
       emitDeclInit(cgf, varDecl, addr);
-
-    if (varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor))
-      errorNYI(varDecl->getSourceRange(), "global with constant storage");
-    else
+      // For constant storage, emit invariant.start in the ctor region after
+      // initialization but before the yield.
+      if (isConstantStorage) {
+        CIRGenBuilderTy &builder = cgf.getBuilder();
+        mlir::OpBuilder::InsertionGuard guard(builder);
+        // Set insertion point to end of ctor region (before yield)
+        if (!addr.getCtorRegion().empty()) {
+          mlir::Block *block = &addr.getCtorRegion().back();
+          // Find the yield op and insert before it
+          mlir::Operation *yieldOp = block->getTerminator();
+          if (yieldOp) {
+            builder.setInsertionPoint(yieldOp);
+            emitDeclInvariant(cgf, varDecl);
+          }
+        }
+      }
+    } else if (isConstantStorage) {
+      emitDeclInvariant(cgf, varDecl);
+    }
+
+    if (!isConstantStorage)
       emitDeclDestroy(cgf, varDecl, addr);
     return;
   }
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index efe0fe5fcc979..627698c5b19af 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -1611,6 +1611,8 @@ class CIRGenFunction : public CIRGenTypeCache {
   mlir::Value emitRuntimeCall(mlir::Location loc, cir::FuncOp callee,
                               llvm::ArrayRef<mlir::Value> args = {});
 
+  void emitInvariantStart(CharUnits Size, mlir::Value Addr, mlir::Location loc);
+
   /// Emit the computation of the specified expression of scalar type.
   mlir::Value emitScalarExpr(const clang::Expr *e,
                              bool ignoreResultAssign = false);
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 1ad1c2fa41aa1..715e877fcca3d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -652,10 +652,19 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty,
 
   mlir::Location loc = getLoc(d->getSourceRange());
 
+  // Calculate constant storage flag before creating the global.
+  bool isConstant = false;
+  if (d) {
+    bool needsDtor =
+        d->needsDestruction(astContext) == QualType::DK_cxx_destructor;
+    isConstant = d->getType().isConstantStorage(
+        astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/!needsDtor);
+  }
+
   // mlir::SymbolTable::Visibility::Public is the default, no need to explicitly
   // mark it as such.
   cir::GlobalOp gv =
-      CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, false,
+      CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, isConstant,
                                    /*insertPoint=*/entry.getOperation());
 
   // This is the first use or definition of a mangled name.  If there is a
@@ -675,10 +684,6 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty,
       errorNYI(d->getSourceRange(), "OpenMP target global variable");
 
     gv.setAlignmentAttr(getSize(astContext.getDeclAlign(d)));
-    // FIXME: This code is overly simple and should be merged with other global
-    // handling.
-    gv.setConstant(d->getType().isConstantStorage(
-        astContext, /*ExcludeCtor=*/false, /*ExcludeDtor=*/false));
 
     setLinkageForGV(gv, d);
 
@@ -870,10 +875,14 @@ void CIRGenModule::emitGlobalVarDefinition(const clang::VarDecl *vd,
     emitter->finalize(gv);
 
   // If it is safe to mark the global 'constant', do so now.
+  // Use the same logic as emitCXXGlobalVarDeclInit to determine constant
+  // storage.
+  bool needsDtor =
+      vd->needsDestruction(astContext) == QualType::DK_cxx_destructor;
   gv.setConstant((vd->hasAttr<CUDAConstantAttr>() && langOpts.CUDAIsDevice) ||
-                 (!needsGlobalCtor && !needsGlobalDtor &&
-                  vd->getType().isConstantStorage(
-                      astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/true)));
+                 vd->getType().isConstantStorage(astContext,
+                                                 /*ExcludeCtor=*/true,
+                                                 /*ExcludeDtor=*/!needsDtor));
   assert(!cir::MissingFeatures::opGlobalSection());
 
   // Set CIR's linkage type as appropriate.
diff --git a/clang/test/CIR/CodeGen/global-constant-storage.cpp b/clang/test/CIR/CodeGen/global-constant-storage.cpp
new file mode 100644
index 0000000000000..8aa240cf8fc5b
--- /dev/null
+++ b/clang/test/CIR/CodeGen/global-constant-storage.cpp
@@ -0,0 +1,244 @@
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt.ll
+// RUN: FileCheck --check-prefix=LLVM-OPT --input-file=%t-opt.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt-ogcg.ll
+// RUN: FileCheck --check-prefix=OGCG-OPT --input-file=%t-opt-ogcg.ll %s
+
+// Test for global with constant storage - const object with constructor but no destructor
+// Check that we add an llvm.invariant.start to mark when a global becomes read-only.
+
+struct A {
+  A();
+  int n;
+};
+
+// Should emit invariant.start - has constructor, no destructor, no mutable
+extern const A a = A();
+
+struct A2 {
+  A2();
+  constexpr ~A2() {}
+  int n;
+};
+
+// Should emit invariant.start - constexpr destructor doesn't prevent constant storage
+extern const A2 a2 = A2();
+
+struct B {
+  B();
+  mutable int n;
+};
+
+// Should NOT emit invariant.start - has mutable member
+extern const B b = B();
+
+struct CWithDtor {
+  CWithDtor();
+  ~CWithDtor();
+  int n;
+};
+
+// Should NOT emit invariant.start - has non-constexpr destructor
+extern const CWithDtor c_with_dtor = CWithDtor();
+
+// Simple case - just const C c; (no initializer) - Andy's suggestion
+class C {
+public:
+  C();
+  int a;
+  int b;
+};
+
+const C c;
+
+// CIR checks for 'a' - should have constant storage
+// CIR: cir.global constant external @a = #cir.zero : !rec_A
+// CIR: cir.func internal private @__cxx_global_var_init() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @a : !cir.ptr<!rec_A>
+// CIR:   cir.call @_ZN1AC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'a2' - should have constant storage (constexpr dtor)
+// CIR: cir.global constant external @a2 = #cir.zero : !rec_A2
+// CIR: cir.func internal private @__cxx_global_var_init.1() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @a2 : !cir.ptr<!rec_A2>
+// CIR:   cir.call @_ZN2A2C1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A2>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'b' - should NOT have constant storage (mutable member)
+// CIR: cir.global external @b = #cir.zero : !rec_B
+// CIR: cir.func internal private @__cxx_global_var_init.2() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @b : !cir.ptr<!rec_B>
+// CIR:   cir.call @_ZN1BC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_B>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'c_with_dtor' - should NOT have constant storage (non-constexpr dtor)
+// CIR: cir.global external @c_with_dtor = #cir.zero : !rec_CWithDtor
+// CIR: cir.func internal private @__cxx_global_var_init.3() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @c_with_dtor : !cir.ptr<!rec_CWithDtor>
+// CIR:   cir.call @_ZN9CWithDtorC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_CWithDtor>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'c' - Andy's simple case, should have constant storage (internal linkage)
+// CIR: cir.global {{.*}} constant internal {{.*}} @_ZL1c = #cir.zero : !rec_C
+// CIR: cir.func internal private @__cxx_global_var_init.4() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @_ZL1c : !cir.ptr<!rec_C>
+// CIR:   cir.call @_ZN1CC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_C>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// LLVM checks (no optimization)
+// Check all globals first (they appear at the top)
+// LLVM: @a ={{.*}} constant {{.*}} zeroinitializer
+// LLVM: @a2 ={{.*}} constant {{.*}} zeroinitializer
+// LLVM: @b ={{.*}} global {{.*}} zeroinitializer
+// LLVM: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// LLVM: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer
+
+// Then check the init functions
+// LLVM: define internal void @__cxx_global_var_init() {
+// LLVM:   call void @_ZN1AC1Ev(ptr @a)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.1() {
+// LLVM:   call void @_ZN2A2C1Ev(ptr @a2)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.2() {
+// LLVM:   call void @_ZN1BC1Ev(ptr @b)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.3() {
+// LLVM:   call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.4() {
+// LLVM:   call void @_ZN1CC1Ev(ptr @_ZL1c)
+// LLVM:   ret void
+// LLVM: }
+
+// OGCG checks (no optimization)
+// Check all globals first (they appear at the top)
+// OGCG: @a ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @a2 ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @b ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @_ZL1c ={{.*}} global {{.*}} zeroinitializer
+
+// Then check the init functions
+// OGCG: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c)
+// OGCG:   ret void
+// OGCG: }
+
+// With optimization enabled, should emit invariant.start intrinsic for constant storage cases
+// Check all globals first (they appear at the top)
+// LLVM-OPT: @a ={{.*}} constant {{.*}} zeroinitializer
+// LLVM-OPT: @a2 ={{.*}} constant {{.*}} zeroinitializer
+// LLVM-OPT: @b ={{.*}} global {{.*}} zeroinitializer
+// LLVM-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// LLVM-OPT: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer
+
+// Then check the init functions with invariant.start
+// LLVM-OPT: define internal void @__cxx_global_var_init() {
+// LLVM-OPT:   call void @_ZN1AC1Ev(ptr @a)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.1() {
+// LLVM-OPT:   call void @_ZN2A2C1Ev(ptr @a2)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.2() {
+// LLVM-OPT:   call void @_ZN1BC1Ev(ptr @b)
+// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.3() {
+// LLVM-OPT:   call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor)
+// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.4() {
+// LLVM-OPT:   call void @_ZN1CC1Ev(ptr @_ZL1c)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// OGCG-OPT checks (with optimization)
+// Check all globals first (they appear at the top)
+// OGCG-OPT: @a ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @a2 ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @b ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @_ZL1c ={{.*}} global {{.*}} zeroinitializer
+
+// Then check the init functions with invariant.start
+// OGCG-OPT: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b)
+// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor)
+// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+

@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2025

@llvm/pr-subscribers-clangir

Author: None (adams381)

Changes

Implement emitDeclInvariant to emit llvm.invariant.start intrinsic for global variables with constant storage. This enables optimizations by marking when a global becomes read-only after initialization.

Changes

  • Add emitDeclInvariant and emitInvariantStart functions in CIRGenCXX.cpp
  • Add emitInvariantStart declaration in CIRGenFunction.h
  • Update emitCXXGlobalVarDeclInit to call emitDeclInvariant for constant storage globals after initialization
  • Update getOrCreateCIRGlobal to set constant flag on globals with constant storage
  • Add comprehensive test covering positive and negative cases

Implementation Details

The implementation handles address spaces correctly, dynamically constructing the intrinsic name (e.g., invariant.start.p0, invariant.start.p10) based on the pointer's address space. The intrinsic is only emitted when optimizations are enabled, matching classic codegen behavior.

Testing

All tests pass (411/412, 1 unsupported). The test file includes CIR, LLVM, and OGCG checks for both optimized and non-optimized builds.

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

4 Files Affected:

  • (modified) clang/lib/CIR/CodeGen/CIRGenCXX.cpp (+67-5)
  • (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+2)
  • (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+17-8)
  • (added) clang/test/CIR/CodeGen/global-constant-storage.cpp (+244)
diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
index 71568ec87a31b..c0890f6b42663 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp
@@ -16,11 +16,54 @@
 
 #include "clang/AST/GlobalDecl.h"
 #include "clang/CIR/MissingFeatures.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
 using namespace clang::CIRGen;
 
+/// Emit code to cause the variable at the given address to be considered as
+/// constant from this point onwards.
+static void emitDeclInvariant(CIRGenFunction &CGF, const VarDecl *D) {
+  mlir::Value addr = CGF.cgm.getAddrOfGlobalVar(D);
+  CGF.emitInvariantStart(CGF.getContext().getTypeSizeInChars(D->getType()),
+                         addr, CGF.getLoc(D->getSourceRange()));
+}
+
+void CIRGenFunction::emitInvariantStart(CharUnits Size, mlir::Value Addr,
+                                        mlir::Location loc) {
+  // Do not emit the intrinsic if we're not optimizing.
+  if (!cgm.getCodeGenOpts().OptimizationLevel)
+    return;
+
+  CIRGenBuilderTy &builder = getBuilder();
+
+  // Create the size constant as i64
+  uint64_t width = Size.getQuantity();
+  mlir::Value sizeValue = builder.getConstInt(loc, builder.getSInt64Ty(),
+                                              static_cast<int64_t>(width));
+
+  // Determine address space for intrinsic name
+  unsigned addrSpace = 0;
+  if (auto ptrTy = mlir::dyn_cast<cir::PointerType>(Addr.getType()))
+    addrSpace =
+        ptrTy.getAddrSpace() ? ptrTy.getAddrSpace().getValue().getUInt() : 0;
+
+  // Format intrinsic name with address space suffix (e.g.,
+  // "invariant.start.p0", "invariant.start.p10")
+  llvm::SmallString<32> intrinsicName;
+  llvm::raw_svector_ostream os(intrinsicName);
+  os << "invariant.start.p" << addrSpace;
+
+  // Create the intrinsic call. The llvm.invariant.start intrinsic returns a
+  // token, but we don't need to capture it. The return type is set to match
+  // the address type for consistency with the operation signature.
+  cir::LLVMIntrinsicCallOp::create(
+      builder, loc, builder.getStringAttr(intrinsicName), Addr.getType(),
+      mlir::ValueRange{sizeValue, Addr});
+}
+
 static void emitDeclInit(CIRGenFunction &cgf, const VarDecl *varDecl,
                          cir::GlobalOp globalOp) {
   assert((varDecl->hasGlobalStorage() ||
@@ -234,13 +277,32 @@ void CIRGenModule::emitCXXGlobalVarDeclInit(const VarDecl *varDecl,
 
     bool needsDtor = varDecl->needsDestruction(getASTContext()) ==
                      QualType::DK_cxx_destructor;
+    bool isConstantStorage =
+        varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor);
     // PerformInit, constant store invariant / destroy handled below.
-    if (performInit)
+    if (performInit) {
       emitDeclInit(cgf, varDecl, addr);
-
-    if (varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor))
-      errorNYI(varDecl->getSourceRange(), "global with constant storage");
-    else
+      // For constant storage, emit invariant.start in the ctor region after
+      // initialization but before the yield.
+      if (isConstantStorage) {
+        CIRGenBuilderTy &builder = cgf.getBuilder();
+        mlir::OpBuilder::InsertionGuard guard(builder);
+        // Set insertion point to end of ctor region (before yield)
+        if (!addr.getCtorRegion().empty()) {
+          mlir::Block *block = &addr.getCtorRegion().back();
+          // Find the yield op and insert before it
+          mlir::Operation *yieldOp = block->getTerminator();
+          if (yieldOp) {
+            builder.setInsertionPoint(yieldOp);
+            emitDeclInvariant(cgf, varDecl);
+          }
+        }
+      }
+    } else if (isConstantStorage) {
+      emitDeclInvariant(cgf, varDecl);
+    }
+
+    if (!isConstantStorage)
       emitDeclDestroy(cgf, varDecl, addr);
     return;
   }
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index efe0fe5fcc979..627698c5b19af 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -1611,6 +1611,8 @@ class CIRGenFunction : public CIRGenTypeCache {
   mlir::Value emitRuntimeCall(mlir::Location loc, cir::FuncOp callee,
                               llvm::ArrayRef<mlir::Value> args = {});
 
+  void emitInvariantStart(CharUnits Size, mlir::Value Addr, mlir::Location loc);
+
   /// Emit the computation of the specified expression of scalar type.
   mlir::Value emitScalarExpr(const clang::Expr *e,
                              bool ignoreResultAssign = false);
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 1ad1c2fa41aa1..715e877fcca3d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -652,10 +652,19 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty,
 
   mlir::Location loc = getLoc(d->getSourceRange());
 
+  // Calculate constant storage flag before creating the global.
+  bool isConstant = false;
+  if (d) {
+    bool needsDtor =
+        d->needsDestruction(astContext) == QualType::DK_cxx_destructor;
+    isConstant = d->getType().isConstantStorage(
+        astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/!needsDtor);
+  }
+
   // mlir::SymbolTable::Visibility::Public is the default, no need to explicitly
   // mark it as such.
   cir::GlobalOp gv =
-      CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, false,
+      CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, isConstant,
                                    /*insertPoint=*/entry.getOperation());
 
   // This is the first use or definition of a mangled name.  If there is a
@@ -675,10 +684,6 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty,
       errorNYI(d->getSourceRange(), "OpenMP target global variable");
 
     gv.setAlignmentAttr(getSize(astContext.getDeclAlign(d)));
-    // FIXME: This code is overly simple and should be merged with other global
-    // handling.
-    gv.setConstant(d->getType().isConstantStorage(
-        astContext, /*ExcludeCtor=*/false, /*ExcludeDtor=*/false));
 
     setLinkageForGV(gv, d);
 
@@ -870,10 +875,14 @@ void CIRGenModule::emitGlobalVarDefinition(const clang::VarDecl *vd,
     emitter->finalize(gv);
 
   // If it is safe to mark the global 'constant', do so now.
+  // Use the same logic as emitCXXGlobalVarDeclInit to determine constant
+  // storage.
+  bool needsDtor =
+      vd->needsDestruction(astContext) == QualType::DK_cxx_destructor;
   gv.setConstant((vd->hasAttr<CUDAConstantAttr>() && langOpts.CUDAIsDevice) ||
-                 (!needsGlobalCtor && !needsGlobalDtor &&
-                  vd->getType().isConstantStorage(
-                      astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/true)));
+                 vd->getType().isConstantStorage(astContext,
+                                                 /*ExcludeCtor=*/true,
+                                                 /*ExcludeDtor=*/!needsDtor));
   assert(!cir::MissingFeatures::opGlobalSection());
 
   // Set CIR's linkage type as appropriate.
diff --git a/clang/test/CIR/CodeGen/global-constant-storage.cpp b/clang/test/CIR/CodeGen/global-constant-storage.cpp
new file mode 100644
index 0000000000000..8aa240cf8fc5b
--- /dev/null
+++ b/clang/test/CIR/CodeGen/global-constant-storage.cpp
@@ -0,0 +1,244 @@
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt.ll
+// RUN: FileCheck --check-prefix=LLVM-OPT --input-file=%t-opt.ll %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt-ogcg.ll
+// RUN: FileCheck --check-prefix=OGCG-OPT --input-file=%t-opt-ogcg.ll %s
+
+// Test for global with constant storage - const object with constructor but no destructor
+// Check that we add an llvm.invariant.start to mark when a global becomes read-only.
+
+struct A {
+  A();
+  int n;
+};
+
+// Should emit invariant.start - has constructor, no destructor, no mutable
+extern const A a = A();
+
+struct A2 {
+  A2();
+  constexpr ~A2() {}
+  int n;
+};
+
+// Should emit invariant.start - constexpr destructor doesn't prevent constant storage
+extern const A2 a2 = A2();
+
+struct B {
+  B();
+  mutable int n;
+};
+
+// Should NOT emit invariant.start - has mutable member
+extern const B b = B();
+
+struct CWithDtor {
+  CWithDtor();
+  ~CWithDtor();
+  int n;
+};
+
+// Should NOT emit invariant.start - has non-constexpr destructor
+extern const CWithDtor c_with_dtor = CWithDtor();
+
+// Simple case - just const C c; (no initializer) - Andy's suggestion
+class C {
+public:
+  C();
+  int a;
+  int b;
+};
+
+const C c;
+
+// CIR checks for 'a' - should have constant storage
+// CIR: cir.global constant external @a = #cir.zero : !rec_A
+// CIR: cir.func internal private @__cxx_global_var_init() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @a : !cir.ptr<!rec_A>
+// CIR:   cir.call @_ZN1AC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'a2' - should have constant storage (constexpr dtor)
+// CIR: cir.global constant external @a2 = #cir.zero : !rec_A2
+// CIR: cir.func internal private @__cxx_global_var_init.1() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @a2 : !cir.ptr<!rec_A2>
+// CIR:   cir.call @_ZN2A2C1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A2>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'b' - should NOT have constant storage (mutable member)
+// CIR: cir.global external @b = #cir.zero : !rec_B
+// CIR: cir.func internal private @__cxx_global_var_init.2() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @b : !cir.ptr<!rec_B>
+// CIR:   cir.call @_ZN1BC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_B>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'c_with_dtor' - should NOT have constant storage (non-constexpr dtor)
+// CIR: cir.global external @c_with_dtor = #cir.zero : !rec_CWithDtor
+// CIR: cir.func internal private @__cxx_global_var_init.3() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @c_with_dtor : !cir.ptr<!rec_CWithDtor>
+// CIR:   cir.call @_ZN9CWithDtorC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_CWithDtor>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// CIR checks for 'c' - Andy's simple case, should have constant storage (internal linkage)
+// CIR: cir.global {{.*}} constant internal {{.*}} @_ZL1c = #cir.zero : !rec_C
+// CIR: cir.func internal private @__cxx_global_var_init.4() {
+// CIR:   %[[OBJ:.*]] = cir.get_global @_ZL1c : !cir.ptr<!rec_C>
+// CIR:   cir.call @_ZN1CC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_C>) -> ()
+// CIR:   cir.return
+// CIR: }
+
+// LLVM checks (no optimization)
+// Check all globals first (they appear at the top)
+// LLVM: @a ={{.*}} constant {{.*}} zeroinitializer
+// LLVM: @a2 ={{.*}} constant {{.*}} zeroinitializer
+// LLVM: @b ={{.*}} global {{.*}} zeroinitializer
+// LLVM: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// LLVM: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer
+
+// Then check the init functions
+// LLVM: define internal void @__cxx_global_var_init() {
+// LLVM:   call void @_ZN1AC1Ev(ptr @a)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.1() {
+// LLVM:   call void @_ZN2A2C1Ev(ptr @a2)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.2() {
+// LLVM:   call void @_ZN1BC1Ev(ptr @b)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.3() {
+// LLVM:   call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor)
+// LLVM:   ret void
+// LLVM: }
+
+// LLVM: define internal void @__cxx_global_var_init.4() {
+// LLVM:   call void @_ZN1CC1Ev(ptr @_ZL1c)
+// LLVM:   ret void
+// LLVM: }
+
+// OGCG checks (no optimization)
+// Check all globals first (they appear at the top)
+// OGCG: @a ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @a2 ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @b ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// OGCG: @_ZL1c ={{.*}} global {{.*}} zeroinitializer
+
+// Then check the init functions
+// OGCG: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor)
+// OGCG:   ret void
+// OGCG: }
+
+// OGCG: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" {
+// OGCG:   call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c)
+// OGCG:   ret void
+// OGCG: }
+
+// With optimization enabled, should emit invariant.start intrinsic for constant storage cases
+// Check all globals first (they appear at the top)
+// LLVM-OPT: @a ={{.*}} constant {{.*}} zeroinitializer
+// LLVM-OPT: @a2 ={{.*}} constant {{.*}} zeroinitializer
+// LLVM-OPT: @b ={{.*}} global {{.*}} zeroinitializer
+// LLVM-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// LLVM-OPT: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer
+
+// Then check the init functions with invariant.start
+// LLVM-OPT: define internal void @__cxx_global_var_init() {
+// LLVM-OPT:   call void @_ZN1AC1Ev(ptr @a)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.1() {
+// LLVM-OPT:   call void @_ZN2A2C1Ev(ptr @a2)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.2() {
+// LLVM-OPT:   call void @_ZN1BC1Ev(ptr @b)
+// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.3() {
+// LLVM-OPT:   call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor)
+// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// LLVM-OPT: define internal void @__cxx_global_var_init.4() {
+// LLVM-OPT:   call void @_ZN1CC1Ev(ptr @_ZL1c)
+// LLVM-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c)
+// LLVM-OPT:   ret void
+// LLVM-OPT: }
+
+// OGCG-OPT checks (with optimization)
+// Check all globals first (they appear at the top)
+// OGCG-OPT: @a ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @a2 ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @b ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer
+// OGCG-OPT: @_ZL1c ={{.*}} global {{.*}} zeroinitializer
+
+// Then check the init functions with invariant.start
+// OGCG-OPT: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b)
+// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor)
+// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+
+// OGCG-OPT: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" {
+// OGCG-OPT:   call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c)
+// OGCG-OPT:   call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c)
+// OGCG-OPT:   ret void
+// OGCG-OPT: }
+

andykaylor
andykaylor reviewed Dec 11, 2025
View reviewed changes
clang/lib/CIR/CodeGen/CIRGenCXX.cpp Outdated

/// Emit code to cause the variable at the given address to be considered as
/// constant from this point onwards.
static void emitDeclInvariant(CIRGenFunction &CGF, const VarDecl *D) {
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

Suggested change
static void emitDeclInvariant(CIRGenFunction &CGF, const VarDecl *D) {
static void emitDeclInvariant(CIRGenFunction &cgf, const VarDecl *d) {

You'll need to get used to the incubator code often having the wrong coding style.

clang/lib/CIR/CodeGen/CIRGenCXX.cpp
// "invariant.start.p0", "invariant.start.p10")
llvm::SmallString<32> intrinsicName;
llvm::raw_svector_ostream os(intrinsicName);
os << "invariant.start.p" << addrSpace;
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

I could be wrong, but I don't think this is necessary. If you call the intrinsic with just the base name, I think the address space should get tacked on automatically when the intrinsic is lowered to LLVM IR.

clang/lib/CIR/CodeGen/CIRGenModule.cpp

mlir::Location loc = getLoc(d->getSourceRange());

// Calculate constant storage flag before creating the global.
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

Why did you move this?

clang/lib/CIR/CodeGen/CIRGenModule.cpp
emitter->finalize(gv);

// If it is safe to mark the global 'constant', do so now.
// Use the same logic as emitCXXGlobalVarDeclInit to determine constant
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

This code does not use the same logic as emitCXXGlobalVarDeclInit in classic codegen. I don't know why it doesn't but it doesn't. This change could lead us to behave differently.

clang/test/CIR/CodeGen/global-constant-storage.cpp
// Should NOT emit invariant.start - has non-constexpr destructor
extern const CWithDtor c_with_dtor = CWithDtor();

// Simple case - just const C c; (no initializer) - Andy's suggestion
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

This is effectively a duplicate of the A case. It looks like CWithDtor is just C in the equivalent classic codegen test. Let's go back to that.

clang/test/CIR/CodeGen/global-constant-storage.cpp

const C c;

// CIR checks for 'a' - should have constant storage
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

It's very hard to compare the CIR, LLVM, and OGCG output in these checks. Can you interleave them so that the various implementations of each case are adjacent?

clang/test/CIR/CodeGen/global-constant-storage.cpp
const C c;

// CIR checks for 'a' - should have constant storage
// CIR: cir.global constant external @a = #cir.zero : !rec_A
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

The initial CIR generated for this case should look like this:

  cir.global external dso_local @a = ctor : !rec_A {
    %0 = cir.get_global @a : !cir.ptr<!rec_A>
    cir.call @_ZN1AC1Ev(%0) : (!cir.ptr<!rec_A>) -> ()
  }

That gets transformed into what you're checking here during LoweringPrepare, but I'd like to see checks for both states. You can do that by passing -mmlir --mlir-print-ir-before=cir-lowering-prepare on the RUN line and capturing the stderr output to a different file. Look at clang/test/CIR/CodeGen/global-init.cpp for an example.

clang/test/CIR/CodeGen/global-constant-storage.cpp
// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt.ll
Copy link
Contributor

@andykaylor andykaylor Dec 11, 2025

Choose a reason for hiding this comment

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

Can you add another RUN line to test the CIR output at -O1?

@adams381
[CIR] Fix parameter naming in emitDeclInvariant
72b9a9c 
Change parameter names from CGF/D to cgf/d to match LLVM coding
standards. Addresses review comments.
@adams381
[CIR] Fix parameter naming in emitInvariantStart
5ecf23d 
Change parameter names from PascalCase (Size, Addr) to lowercase
(size, addr) to match LLVM coding standards for function parameters.
@adams381
Copy link
Contributor Author

adams381 commented Dec 16, 2025

Addressed parameter naming compliance issue: Changed function parameters in emitInvariantStart from PascalCase (Size, Addr) to lowercase camelCase (size, addr) to match LLVM coding standards for function parameters. This ensures consistency with the rest of the codebase where function parameters use lowercase camelCase naming convention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andykaylor andykaylor andykaylor left review comments

@lanza lanza Awaiting requested review from lanza lanza is a code owner

@bcardosolopes bcardosolopes Awaiting requested review from bcardosolopes bcardosolopes is a code owner

@xlauko xlauko Awaiting requested review from xlauko xlauko is a code owner

Assignees

No one assigned

Labels

clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adams381 @llvmbot @andykaylor