[clang] 8ea2b58 - [CIR] Avoid duplicate name collisions in LoweringPrepare (#194469)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 28 09:50:24 PDT 2026


Author: Andy Kaylor
Date: 2026-04-28T09:50:19-07:00
New Revision: 8ea2b587c0f0a430ae0e7563e62e79c39b675683

URL: https://github.com/llvm/llvm-project/commit/8ea2b587c0f0a430ae0e7563e62e79c39b675683
DIFF: https://github.com/llvm/llvm-project/commit/8ea2b587c0f0a430ae0e7563e62e79c39b675683.diff

LOG: [CIR] Avoid duplicate name collisions in LoweringPrepare (#194469)

This fixes a bug in the CIR LoweringPrepare pass where we were creating
multiple constant initializer global values with the same name, causing
references to them (specifically cir.get_global) to get the wrong value.

Assisted-by: Cursor / claude-4.7-opus-xhigh

Added: 
    clang/test/CIR/CodeGen/local-const-aggregate-name-clash.cpp

Modified: 
    clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index fc5606a9d13f1..bd3c8bc0aa8d1 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -93,10 +93,24 @@ struct LoweringPreparePass
   void lowerArrayDtor(cir::ArrayDtor op);
   void lowerArrayCtor(cir::ArrayCtor op);
   void lowerTrivialCopyCall(cir::CallOp op);
-  void lowerStoreOfConstAggregate(cir::StoreOp op);
+  void lowerStoreOfConstAggregate(cir::StoreOp op,
+                                  mlir::SymbolTableCollection &symbolTables);
   void lowerLocalInitOp(cir::LocalInitOp op,
                         mlir::SymbolTableCollection &symbolTables);
 
+  /// Return a private constant cir::GlobalOp with the given type and initial
+  /// value, suitable for backing a memcpy-initialized local aggregate.
+  ///
+  /// If a global with `baseName` (or one of its `.<n>` versioned siblings)
+  /// already has a matching type and initial value, that global is reused.
+  /// Otherwise a new global is created with the next available `.<n>` suffix
+  /// (matching CIRGenBuilder::createVersionedGlobal and OGCG behavior).
+  cir::GlobalOp
+  getOrCreateConstAggregateGlobal(CIRBaseBuilderTy &builder,
+                                  mlir::SymbolTableCollection &symbolTables,
+                                  mlir::Location loc, llvm::StringRef baseName,
+                                  mlir::Type ty, mlir::TypedAttr constant);
+
   /// Build the function that initializes the specified global
   cir::FuncOp buildCXXGlobalVarDeclInitFunc(cir::GlobalOp op);
 
@@ -220,6 +234,8 @@ struct LoweringPreparePass
   /// Tracks guard variables for static locals (keyed by global symbol name).
   llvm::StringMap<cir::GlobalOp> staticLocalDeclGuardMap;
 
+  llvm::StringMap<llvm::SmallVector<cir::GlobalOp, 1>> constAggregateGlobals;
+
   /// List of ctors and their priorities to be called before main()
   llvm::SmallVector<std::pair<std::string, uint32_t>, 4> globalCtorList;
   /// List of dtors and their priorities to be called when unloading module.
@@ -1679,7 +1695,67 @@ void LoweringPreparePass::lowerTrivialCopyCall(cir::CallOp op) {
   }
 }
 
-void LoweringPreparePass::lowerStoreOfConstAggregate(cir::StoreOp op) {
+cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
+    CIRBaseBuilderTy &builder, mlir::SymbolTableCollection &symbolTables,
+    mlir::Location loc, llvm::StringRef baseName, mlir::Type ty,
+    mlir::TypedAttr constant) {
+  // Look up (and lazily populate) the per-base-name cache.
+  llvm::SmallVector<cir::GlobalOp, 1> &versions =
+      constAggregateGlobals[baseName];
+
+  // First, check globals we've already discovered for this base name.
+  for (cir::GlobalOp gv : versions) {
+    if (gv.getSymType() == ty && gv.getInitialValue() == constant)
+      return gv;
+  }
+
+  // No cached match. Scan the module's symbol table starting from the next
+  // unscanned version. In practice this should usually exit on the first
+  // iteration, but it's possible that some other pass or a previous
+  // invocation of this pass created globals using this same logic.
+  llvm::SmallString<128> name(baseName);
+  size_t baseLen = name.size();
+  unsigned version = versions.size();
+  while (true) {
+    name.resize(baseLen);
+    if (version != 0) {
+      name.push_back('.');
+      llvm::Twine(version).toVector(name);
+    }
+    auto existingGv = symbolTables.lookupSymbolIn<cir::GlobalOp>(
+        mlirModule, mlir::StringAttr::get(&getContext(), name));
+    if (!existingGv)
+      break;
+    versions.push_back(existingGv);
+    if (existingGv.getSymType() == ty &&
+        existingGv.getInitialValue() == constant)
+      return existingGv;
+    ++version;
+  }
+
+  // No match found, create a new global. The loop above found an unused name.
+  mlir::OpBuilder::InsertionGuard guard(builder);
+  builder.setInsertionPointToStart(mlirModule.getBody());
+  auto gv =
+      cir::GlobalOp::create(builder, loc, name, ty,
+                            /*isConstant=*/true,
+                            cir::LangAddressSpaceAttr::get(
+                                &getContext(), cir::LangAddressSpace::Default),
+                            cir::GlobalLinkageKind::PrivateLinkage);
+  mlir::SymbolTable::setSymbolVisibility(
+      gv, mlir::SymbolTable::Visibility::Private);
+  gv.setInitialValueAttr(constant);
+
+  // Keep the cached symbol table in sync with the new global so subsequent
+  // lookups for other base names find it.
+  symbolTables.getSymbolTable(mlirModule).insert(gv);
+
+  versions.push_back(gv);
+  return gv;
+}
+
+void LoweringPreparePass::lowerStoreOfConstAggregate(
+    cir::StoreOp op, mlir::SymbolTableCollection &symbolTables) {
   // Check if the value operand is a cir.const with aggregate type.
   auto constOp = op.getValue().getDefiningOp<cir::ConstantOp>();
   if (!constOp)
@@ -1715,36 +1791,21 @@ void LoweringPreparePass::lowerStoreOfConstAggregate(cir::StoreOp op) {
   // Get variable name from the alloca.
   llvm::StringRef varName = alloca.getName();
 
-  // Build name: __const.<func>.<var>
-  std::string name = ("__const." + funcName + "." + varName).str();
-
-  // Create the global constant.
+  // Build base name: __const.<func>.<var>
+  std::string baseName = ("__const." + funcName + "." + varName).str();
   CIRBaseBuilderTy builder(getContext());
 
-  // Use InsertionGuard to create the global at module level.
-  builder.setInsertionPointToStart(mlirModule.getBody());
-
-  // If a global with this name already exists (e.g. CIRGen materializes
-  // constexpr locals as globals when their address is taken), reuse it.
-  if (!mlir::SymbolTable::lookupSymbolIn(
-          mlirModule, mlir::StringAttr::get(&getContext(), name))) {
-    auto gv = cir::GlobalOp::create(
-        builder, op.getLoc(), name, ty,
-        /*isConstant=*/true,
-        cir::LangAddressSpaceAttr::get(&getContext(),
-                                       cir::LangAddressSpace::Default),
-        cir::GlobalLinkageKind::PrivateLinkage);
-    mlir::SymbolTable::setSymbolVisibility(
-        gv, mlir::SymbolTable::Visibility::Private);
-    gv.setInitialValueAttr(constant);
-  }
+  // Check for existing globals and create a new global with a unique name
+  // if no match is found.
+  cir::GlobalOp gv = getOrCreateConstAggregateGlobal(
+      builder, symbolTables, op.getLoc(), baseName, ty, constant);
 
   // Now replace the store with get_global + copy.
   builder.setInsertionPoint(op);
 
   auto ptrTy = cir::PointerType::get(ty);
   mlir::Value globalPtr =
-      cir::GetGlobalOp::create(builder, op.getLoc(), ptrTy, name);
+      cir::GetGlobalOp::create(builder, op.getLoc(), ptrTy, gv.getSymName());
 
   // Replace store with copy.
   builder.createCopy(op.getAddr(), globalPtr);
@@ -1776,7 +1837,7 @@ void LoweringPreparePass::runOnOp(mlir::Operation *op,
   } else if (auto callOp = dyn_cast<cir::CallOp>(op)) {
     lowerTrivialCopyCall(callOp);
   } else if (auto storeOp = dyn_cast<cir::StoreOp>(op)) {
-    lowerStoreOfConstAggregate(storeOp);
+    lowerStoreOfConstAggregate(storeOp, symbolTables);
   } else if (auto fnOp = dyn_cast<cir::FuncOp>(op)) {
     if (auto globalCtor = fnOp.getGlobalCtorPriority())
       globalCtorList.emplace_back(fnOp.getName(), globalCtor.value());

diff  --git a/clang/test/CIR/CodeGen/local-const-aggregate-name-clash.cpp b/clang/test/CIR/CodeGen/local-const-aggregate-name-clash.cpp
new file mode 100644
index 0000000000000..c4c6b0d466beb
--- /dev/null
+++ b/clang/test/CIR/CodeGen/local-const-aggregate-name-clash.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll
+// RUN: FileCheck --input-file=%t-cir.ll %s --check-prefix=LLVM
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --input-file=%t.ll %s --check-prefix=OGCG
+
+// Two distinct local aggregates in the same function may share a name (for
+// example, arrays declared in 
diff erent blocks). The per-function constant
+// globals (@__const.<func>.<var>) materialized to back memcpy initialization
+// must be disambiguated with a version suffix so that each cir.get_global
+// agrees with the type and value of the global it references.
+
+void use_array(const int *p, int n);
+
+void f(bool which) {
+  if (which) {
+    int arr[] = {10, 20, 30, 40};
+    use_array(arr, 4);
+  } else {
+    int arr[] = {50, 60};
+    use_array(arr, 2);
+  }
+}
+
+// CIR-DAG: cir.global "private" constant cir_private @[[GV0:.*]] = #cir.const_array<[#cir.int<10> : !s32i, #cir.int<20> : !s32i, #cir.int<30> : !s32i, #cir.int<40> : !s32i]> : !cir.array<!s32i x 4>
+// CIR-DAG: cir.global "private" constant cir_private @[[GV1:.*]] = #cir.const_array<[#cir.int<50> : !s32i, #cir.int<60> : !s32i]> : !cir.array<!s32i x 2>
+
+// CIR: cir.func{{.*}} @_Z1fb
+// CIR:   cir.get_global @[[GV0]] : !cir.ptr<!cir.array<!s32i x 4>>
+// CIR:   cir.get_global @[[GV1]] : !cir.ptr<!cir.array<!s32i x 2>>
+
+// LLVM-DAG: @[[GV0:.*]] = private constant [4 x i32] [i32 10, i32 20, i32 30, i32 40]
+// LLVM-DAG: @[[GV1:.*]] = private constant [2 x i32] [i32 50, i32 60]
+
+// LLVM: define{{.*}} @_Z1fb
+// LLVM:   call void @llvm.memcpy.p0.p0.i64(ptr {{[^,]+}}, ptr @[[GV0]], i64 16, i1 false)
+// LLVM:   call void @llvm.memcpy.p0.p0.i64(ptr {{[^,]+}}, ptr @[[GV1]], i64 8, i1 false)
+
+// OGCG-DAG: @[[GV0:.*]] = private unnamed_addr constant [4 x i32] [i32 10, i32 20, i32 30, i32 40]
+// OGCG-DAG: @[[GV1:.*]] = private unnamed_addr constant [2 x i32] [i32 50, i32 60]
+
+// OGCG: define{{.*}} @_Z1fb
+// OGCG:   call void @llvm.memcpy.p0.p0.i64(ptr {{[^,]+}}, ptr {{[^,]+}}@[[GV0]], i64 16, i1 false)
+// OGCG:   call void @llvm.memcpy.p0.p0.i64(ptr {{[^,]+}}, ptr {{[^,]+}}@[[GV1]], i64 8, i1 false)


        


More information about the cfe-commits mailing list