[CIR] Allow multi-block ctor regions on GlobalOp#193596
Merged
Conversation
adams381
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: adams381 ChangesC++14 variable templates with non-constexpr constructors (e.g., The problem is that This patch relaxes the constraint to Made with Cursor Full diff: https://github.com/llvm/llvm-project/pull/193596.diff 4 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index f20ba262d6480..8179610a4a33e 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -2865,8 +2865,8 @@ def CIR_GlobalOp : CIR_Op<"global", [
OptionalAttr<StrAttr>:$section
);
- let regions = (region MaxSizedRegion<1>:$ctorRegion,
- MaxSizedRegion<1>:$dtorRegion);
+ let regions = (region AnyRegion:$ctorRegion,
+ AnyRegion:$dtorRegion);
let assemblyFormat = [{
($sym_visibility^)?
diff --git a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
index ec032a92591d7..5d99d5b2f6ad3 100644
--- a/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/LoweringPrepare.cpp
@@ -193,8 +193,7 @@ struct LoweringPreparePass
globalOp->emitError("NYI: guard COMDAT for non-local variables");
return {};
} else if (hasComdat && globalOp.isWeakForLinker()) {
- globalOp->emitError("NYI: guard COMDAT for weak linkage");
- return {};
+ guard.setComdat(true);
}
setStaticLocalDeclGuardAddress(globalSymName, guard);
@@ -290,8 +289,9 @@ struct LoweringPreparePass
// Emit the initializer and add a global destructor if appropriate.
auto &ctorRegion = globalOp.getCtorRegion();
assert(!ctorRegion.empty() && "This should never be empty here.");
- if (!ctorRegion.hasOneBlock())
- llvm_unreachable("Multiple blocks NYI");
+ assert(ctorRegion.hasOneBlock() &&
+ "Static local ctor regions with multiple blocks should use "
+ "LocalInitOp (see PR #193576)");
mlir::Block &block = ctorRegion.front();
mlir::Block *insertBlock = builder.getInsertionBlock();
insertBlock->getOperations().splice(insertBlock->end(),
@@ -982,7 +982,12 @@ LoweringPreparePass::buildCXXGlobalVarDeclInitFunc(cir::GlobalOp op) {
FuncOp f = buildRuntimeFunction(builder, fnName, op.getLoc(), fnType,
cir::GlobalLinkageKind::InternalLinkage);
- // Move over the initialzation code of the ctor region.
+ // Move over the initialization code of the ctor region.
+ // The ctor region may have multiple blocks when exception handling
+ // scaffolding creates extra blocks (e.g., unreachable/trap blocks).
+ // We move all operations from the first block (minus the yield) into
+ // the function entry, and discard extra blocks (which contain only
+ // unreachable terminators from EH cleanup paths).
mlir::Block *entryBB = f.addEntryBlock();
if (!op.getCtorRegion().empty()) {
mlir::Block &block = op.getCtorRegion().front();
diff --git a/clang/test/CIR/CodeGen/global-var-template-ctor.cpp b/clang/test/CIR/CodeGen/global-var-template-ctor.cpp
new file mode 100644
index 0000000000000..a01d7d7431a32
--- /dev/null
+++ b/clang/test/CIR/CodeGen/global-var-template-ctor.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -std=c++14 -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++14 -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++14 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
+// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+
+// Regression test: C++14 variable templates with non-constexpr
+// constructors must not crash CIR with "ctorRegion failed to
+// verify constraint: region with at most 1 blocks".
+
+template<int N> class FixedInt {
+public:
+ static const int value = N;
+ operator int() const { return value; }
+ FixedInt() {}
+};
+
+template<int N>
+static const FixedInt<N> fix{};
+
+int test() {
+ return fix<1> + fix<2>;
+}
+
+// CIR: cir.global "private" internal dso_local @_ZL3fixILi1EE
+// CIR: cir.global "private" internal dso_local @_ZL3fixILi2EE
+// CIR: cir.func {{.*}} @_Z4testv
+
+// LLVM: @_ZL3fixILi1EE = internal global
+// LLVM: @_ZL3fixILi2EE = internal global
+// LLVM: define {{.*}} @_Z4testv
+
+// OGCG: @_ZL3fixILi1EE = internal global
+// OGCG: @_ZL3fixILi2EE = internal global
+// OGCG: define {{.*}} @_Z4testv
diff --git a/clang/test/CIR/CodeGen/static-local.cpp b/clang/test/CIR/CodeGen/static-local.cpp
index 2de140ee6d07e..615608a1928c9 100644
--- a/clang/test/CIR/CodeGen/static-local.cpp
+++ b/clang/test/CIR/CodeGen/static-local.cpp
@@ -28,6 +28,23 @@ void f() {
use(&a);
}
+// Static local in an inline function: the variable and guard both get
+// linkonce_odr linkage and their own COMDAT groups.
+void use(const A *);
+inline const A &getInlineA() {
+ static A a;
+ return a;
+}
+
+void call_inline() {
+ use(&getInlineA());
+}
+
+// CIR-BEFORE-LPP: cir.global linkonce_odr comdat static_local_guard<"_ZGVZ10getInlineAvE1a"> @_ZZ10getInlineAvE1a = ctor : !rec_A {
+// CIR-BEFORE-LPP: %[[ADDR2:.*]] = cir.get_global static_local @_ZZ10getInlineAvE1a : !cir.ptr<!rec_A>
+// CIR-BEFORE-LPP: cir.call @_ZN1AC1Ev(%[[ADDR2]]) : (!cir.ptr<!rec_A> {{.*}}) -> ()
+// CIR-BEFORE-LPP: }
+
// CIR-BEFORE-LPP: cir.global "private" internal dso_local static_local_guard<"_ZGVZ1fvE1a"> @_ZZ1fvE1a = ctor : !rec_A {
// CIR-BEFORE-LPP: %[[ADDR:.*]] = cir.get_global static_local @_ZZ1fvE1a : !cir.ptr<!rec_A>
// CIR-BEFORE-LPP: cir.call @_ZN1AC1Ev(%[[ADDR]]) : (!cir.ptr<!rec_A> {{.*}}) -> ()
@@ -39,6 +56,16 @@ void f() {
// CIR-BEFORE-LPP: cir.return
// CIR-DAG: cir.global "private" internal dso_local @_ZGVZ1fvE1a = #cir.int<0> : !s64i
+// CIR-DAG: cir.global "private" linkonce_odr comdat @_ZGVZ10getInlineAvE1a = #cir.int<0> : !s64i
+
+// LLVM-DAG: @_ZGVZ1fvE1a = internal global i64 0
+// LLVM-DAG: @_ZZ10getInlineAvE1a = linkonce_odr global %class.A zeroinitializer, comdat, align 1
+// LLVM-DAG: @_ZGVZ10getInlineAvE1a = linkonce_odr global i64 0, comdat, align 8
+
+// OGCG-DAG: @_ZGVZ1fvE1a = internal global i64 0
+// OGCG-DAG: @_ZZ10getInlineAvE1a = linkonce_odr global %class.A zeroinitializer, comdat, align 1
+// OGCG-DAG: @_ZGVZ10getInlineAvE1a = linkonce_odr global i64 0, comdat, align 8
+
// CIR: cir.func{{.*}}@_Z1fv()
// CIR: %[[ADDR:.*]] = cir.get_global static_local @_ZZ1fvE1a : !cir.ptr<!rec_A>
// CIR: %[[GUARD:.*]] = cir.get_global @_ZGVZ1fvE1a : !cir.ptr<!s64i>
@@ -54,7 +81,19 @@ void f() {
// CIR: cir.call @_Z3useP1A(%[[ADDR]])
// CIR: cir.return
-// LLVM-DAG: @_ZGVZ1fvE1a = internal global i64 0
+// CIR: cir.func{{.*}}@_Z10getInlineAv()
+// CIR: %[[ADDR2:.*]] = cir.get_global static_local @_ZZ10getInlineAvE1a : !cir.ptr<!rec_A>
+// CIR: %[[GUARD2:.*]] = cir.get_global @_ZGVZ10getInlineAvE1a : !cir.ptr<!s64i>
+// CIR: %[[GUARD_BYTE_PTR2:.*]] = cir.cast bitcast %[[GUARD2]] : !cir.ptr<!s64i> -> !cir.ptr<!s8i>
+// CIR: %[[GUARD_LOAD2:.*]] = cir.load{{.*}}%[[GUARD_BYTE_PTR2]]
+// CIR: %[[ZERO2:.*]] = cir.const #cir.int<0>
+// CIR: %[[IS_UNINIT2:.*]] = cir.cmp eq %[[GUARD_LOAD2]], %[[ZERO2]]
+// CIR: cir.if %[[IS_UNINIT2]]
+// CIR: cir.call @__cxa_guard_acquire
+// CIR: cir.if
+// CIR: cir.call @_ZN1AC1Ev
+// CIR: cir.call @__cxa_guard_release
+
// LLVM: define{{.*}}void @_Z1fv()
// LLVM: %[[GUARD:.*]] = load atomic i8, ptr @_ZGVZ1fvE1a acquire
// LLVM: %[[IS_UNINIT:.*]] = icmp eq i8 %[[GUARD]], 0
@@ -65,7 +104,14 @@ void f() {
// LLVM: call void @_Z3useP1A(ptr {{.*}}@_ZZ1fvE1a)
// LLVM: ret void
-// OGCG-DAG: @_ZGVZ1fvE1a = internal global i64 0
+// LLVM: define linkonce_odr {{.*}}ptr @_Z10getInlineAv()
+// LLVM: %[[GUARD3:.*]] = load atomic i8, ptr @_ZGVZ10getInlineAvE1a acquire
+// LLVM: %[[IS_UNINIT3:.*]] = icmp eq i8 %[[GUARD3]], 0
+// LLVM: br i1 %[[IS_UNINIT3]]
+// LLVM: call i32 @__cxa_guard_acquire
+// LLVM: call void @_ZN1AC1Ev
+// LLVM: call void @__cxa_guard_release
+
// OGCG: define{{.*}}void @_Z1fv()
// OGCG: %[[GUARD:.*]] = load atomic i8, ptr @_ZGVZ1fvE1a acquire
// OGCG: %[[IS_UNINIT:.*]] = icmp eq i8 %[[GUARD]], 0
@@ -75,3 +121,11 @@ void f() {
// OGCG: call void @__cxa_guard_release
// OGCG: call void @_Z3useP1A(ptr {{.*}}@_ZZ1fvE1a)
// OGCG: ret void
+
+// OGCG: define linkonce_odr {{.*}}ptr @_Z10getInlineAv() {{.*}}comdat
+// OGCG: %[[GUARD3:.*]] = load atomic i8, ptr @_ZGVZ10getInlineAvE1a acquire
+// OGCG: %[[IS_UNINIT3:.*]] = icmp eq i8 %[[GUARD3]], 0
+// OGCG: br i1 %[[IS_UNINIT3]]
+// OGCG: call i32 @__cxa_guard_acquire
+// OGCG: call void @_ZN1AC1Ev
+// OGCG: call void @__cxa_guard_release
|
andykaylor
reviewed
Apr 22, 2026
Contributor
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
It looks like this needs to be rebased. Otherwise, it looks good to me.
| llvm_unreachable("Multiple blocks NYI"); | ||
| assert(ctorRegion.hasOneBlock() && | ||
| "Static local ctor regions with multiple blocks should use " | ||
| "LocalInitOp (see PR #193576)"); |
Contributor
There was a problem hiding this comment.
I'm not sure it's useful to mention the PR number in the assert. It won't be useful after that is merged.
Contributor
Author
There was a problem hiding this comment.
Good point, removed. Updated to just say "Multi-block static local ctor regions are not yet supported".
adams381
force-pushed
the
cir-fix-ctorregion-multiblock
branch
from
726d5fb to
556e1d1
Compare
Relax the MaxSizedRegion<1> constraint on GlobalOp's ctorRegion and dtorRegion to AnyRegion. When CIRGen emits initialization code for namespace-scope variable templates with non-constexpr constructors, exception handling cleanup paths can create additional blocks in the ctor region. The extra blocks contain only unreachable/trap terminators and are safely discarded when LoweringPrepare moves the init code into __cxx_global_var_init. Made-with: Cursor
adams381
force-pushed
the
cir-fix-ctorregion-multiblock
branch
from
556e1d1 to
8fa7f7e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
C++14 variable templates with non-constexpr constructors (e.g.,
static const Foo<N> x{}whereFoo()is not constexpr) crash CIR codegen with:The problem is that
GlobalOp'sctorRegionis declared asMaxSizedRegion<1>, but when exceptions are enabled,emitAggExprcan create additional blocks in the ctor region for EH cleanup scaffolding (unreachable/trap terminators). These extra blocks are dead code —LoweringPreparealready discards them when it moves the ctor region into__cxx_global_var_init.This patch relaxes the constraint to
AnyRegionand replaces thellvm_unreachable("Multiple blocks NYI")in the static-local guard path with an assert (that path will be reworked by #193576).Made with Cursor