[clang] [CIR] Avoid duplicate name collisions in LoweringPrepare (PR #194469)
Andy Kaylor via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 27 17:48:27 PDT 2026
https://github.com/andykaylor updated https://github.com/llvm/llvm-project/pull/194469
>From fd0e8f02949f12858a376e9d70869130e0a88668 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akaylor at nvidia.com>
Date: Mon, 27 Apr 2026 14:15:05 -0700
Subject: [PATCH 1/3] [CIR] Avoid duplicate name collisions in LoweringPrepare
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
---
.../Dialect/Transforms/LoweringPrepare.cpp | 99 ++++++++++++++-----
.../local-const-aggregate-name-clash.cpp | 45 +++++++++
2 files changed, 122 insertions(+), 22 deletions(-)
create mode 100644 clang/test/CIR/CodeGen/local-const-aggregate-name-clash.cpp
diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index fc5606a9d13f1..782a7a7471009 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -97,6 +97,19 @@ struct LoweringPreparePass
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::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 +233,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,6 +1694,61 @@ void LoweringPreparePass::lowerTrivialCopyCall(cir::CallOp op) {
}
}
+cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
+ CIRBaseBuilderTy &builder, 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 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 = dyn_cast_or_null<cir::GlobalOp>(
+ mlir::SymbolTable::lookupSymbolIn(
+ 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);
+
+ versions.push_back(gv);
+ return gv;
+}
+
void LoweringPreparePass::lowerStoreOfConstAggregate(cir::StoreOp op) {
// Check if the value operand is a cir.const with aggregate type.
auto constOp = op.getValue().getDefiningOp<cir::ConstantOp>();
@@ -1715,36 +1785,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, 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);
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 different 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)
>From 4cecf54dc3c107d989120d72c587001f5d1c77b9 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akaylor at nvidia.com>
Date: Mon, 27 Apr 2026 15:46:14 -0700
Subject: [PATCH 2/3] Use symbolTables
---
.../Dialect/Transforms/LoweringPrepare.cpp | 44 +++++++++++--------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index 782a7a7471009..a0a87d039032d 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -93,7 +93,8 @@ 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);
@@ -104,11 +105,11 @@ struct LoweringPreparePass
/// 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::Location loc,
- llvm::StringRef baseName,
- mlir::Type ty,
- mlir::TypedAttr constant);
+ 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);
@@ -1695,8 +1696,9 @@ void LoweringPreparePass::lowerTrivialCopyCall(cir::CallOp op) {
}
cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
- CIRBaseBuilderTy &builder, mlir::Location loc, llvm::StringRef baseName,
- mlir::Type ty, mlir::TypedAttr constant) {
+ 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];
@@ -1707,10 +1709,10 @@ cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
return gv;
}
- // No cached match. Scan the 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.
+ // 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();
@@ -1720,9 +1722,8 @@ cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
name.push_back('.');
llvm::Twine(version).toVector(name);
}
- auto existingGv = dyn_cast_or_null<cir::GlobalOp>(
- mlir::SymbolTable::lookupSymbolIn(
- mlirModule, mlir::StringAttr::get(&getContext(), name)));
+ auto existingGv = symbolTables.lookupSymbolIn<cir::GlobalOp>(
+ mlirModule, mlir::StringAttr::get(&getContext(), name));
if (!existingGv)
break;
versions.push_back(existingGv);
@@ -1745,11 +1746,16 @@ cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
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) {
+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)
@@ -1791,8 +1797,8 @@ void LoweringPreparePass::lowerStoreOfConstAggregate(cir::StoreOp op) {
// Check for existing globals and create a new global with a unique name
// if no match is found.
- cir::GlobalOp gv = getOrCreateConstAggregateGlobal(builder, op.getLoc(),
- baseName, ty, constant);
+ cir::GlobalOp gv = getOrCreateConstAggregateGlobal(
+ builder, symbolTables, op.getLoc(), baseName, ty, constant);
// Now replace the store with get_global + copy.
builder.setInsertionPoint(op);
@@ -1831,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());
>From 6fdd47870f64eb6fd540484a782b5d3f2f017ce6 Mon Sep 17 00:00:00 2001
From: Andy Kaylor <akaylor at nvidia.com>
Date: Mon, 27 Apr 2026 16:36:01 -0700
Subject: [PATCH 3/3] Fix formatting
---
clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index a0a87d039032d..bd3c8bc0aa8d1 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -1736,12 +1736,12 @@ cir::GlobalOp LoweringPreparePass::getOrCreateConstAggregateGlobal(
// 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);
+ 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);
More information about the cfe-commits
mailing list