Skip to content

[CIR][CodeGen] Implement popNullFixups and unreachable code null fixups #2108

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

Draft
bruteforceboy wants to merge 2 commits into
base: main
Choose a base branch
from
+184 −43
Draft

[CIR][CodeGen] Implement popNullFixups and unreachable code null fixups #2108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d27d38
Remove trap. Use proper insertion point logic.
bruteforceboy Jan 23, 2026
da492a1
add foo OGCG output
bruteforceboy Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ cir::BrOp CIRGenFunction::emitBranchThroughCleanup(mlir::Location Loc,
Fixup.destinationIndex = Dest.getDestIndex();
Fixup.initialBranch = brOp;
Fixup.optimisticBranchBlock = nullptr;
// FIXME(cir): should we clear insertion point here?
insertPointSet = false;
return brOp;
}

Expand Down Expand Up @@ -889,7 +889,15 @@ void EHScopeStack::deallocate(size_t Size) {
void EHScopeStack::popNullFixups() {
// We expect this to only be called when there's still an innermost
// normal cleanup; otherwise there really shouldn't be any fixups.
llvm_unreachable("NYI");
assert(hasNormalCleanups());

EHScopeStack::iterator it = find(InnermostNormalCleanup);
unsigned minSize = cast<EHCleanupScope>(*it).getFixupDepth();
assert(BranchFixups.size() >= minSize && "fixup stack out of order");

while (BranchFixups.size() > minSize &&
BranchFixups.back().destination == nullptr)
BranchFixups.pop_back();
}

bool EHScopeStack::requiresLandingPad() const {
Expand Down
15 changes: 11 additions & 4 deletions clang/lib/CIR/CodeGen/CIRGenDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,14 @@ void CIRGenFunction::emitAutoVarInit(const AutoVarEmission &emission) {
// If this local has an initializer, emit it now.
const Expr *Init = D.getInit();

// TODO: in LLVM codegen if we are at an unreachable point, the initializer
// isn't emitted unless it contains a label. What we want for CIR?
if (!HaveInsertPoint()) {
if (!Init || !ContainsLabel(Init)) {
// TODO(cir): PGO->markStmtMaybeUsed(Init);
return;
}
ensureInsertPoint();
}

assert(builder.getInsertionBlock());

// Initialize the variable here if it doesn't have a initializer and it is a
Expand Down Expand Up @@ -373,8 +379,9 @@ void CIRGenFunction::emitAutoVarCleanups(const AutoVarEmission &emission) {
if (emission.wasEmittedAsGlobal())
return;

// TODO: in LLVM codegen if we are at an unreachable point codgen
// is ignored. What we want for CIR?
if (!HaveInsertPoint())
return;

assert(builder.getInsertionBlock());
const VarDecl &D = *emission.Variable;

Expand Down
3 changes: 2 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ CIRGenFunction::CIRGenFunction(CIRGenModule &cgm, CIRGenBuilderTy &builder,
bool suppressNewContext)
: CIRGenTypeCache(cgm), CGM{cgm}, builder(builder),
SanOpts(cgm.getLangOpts().Sanitize), CurFPFeatures(cgm.getLangOpts()),
ShouldEmitLifetimeMarkers(false) {
ShouldEmitLifetimeMarkers(false), insertPointSet(true) {
if (!suppressNewContext)
cgm.getCXXABI().getMangleContext().startNewFunction();
EHStack.setCGF(this);
Expand Down Expand Up @@ -859,6 +859,7 @@ cir::FuncOp CIRGenFunction::generateCode(clang::GlobalDecl gd, cir::FuncOp fn,
llvm_unreachable("no definition for emitted function");
}

ensureInsertPoint();
assert(builder.getInsertionBlock() && "Should be valid");

if (mlir::failed(fn.verifyBody()))
Expand Down
24 changes: 19 additions & 5 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,10 @@ class CIRGenFunction : public CIRGenTypeCache {
/// the constructor, but could be overwrriten to true if this is a coroutine.
bool ShouldEmitLifetimeMarkers;

/// True if an insertion point is defined. If not, this indicates that the
/// current code being emitted is unreachable.
bool insertPointSet;

/// True if there are any operations in the body of the function that are
/// likely to throw an exception.
bool mayThrow = false;
Expand Down Expand Up @@ -614,11 +618,20 @@ class CIRGenFunction : public CIRGenTypeCache {

/// True if an insertion point is defined. If not, this indicates that the
/// current code being emitted is unreachable.
/// FIXME(cir): we need to inspect this and perhaps use a cleaner mechanism
/// since we don't yet force null insertion point to designate behavior (like
/// LLVM's codegen does) and we probably shouldn't.
bool HaveInsertPoint() const {
return builder.getInsertionBlock() != nullptr;
/// In LLVM's codegen, forcing null insertion points is used to designate
/// behavior. In CIR, we use the `insertPointSet` flag as a mechanism to
/// implement the same behavior, since we don't force null insert points.
bool HaveInsertPoint() const { return insertPointSet; }

/// ensureInsertPoint - Ensure that an insertion point is defined so that
/// emitted IR has a place to go. Note that by definition, if this function
/// creates a block then that block is unreachable; callers may do better to
/// detect when no insertion point is defined and simply skip IR generation.
void ensureInsertPoint() {
if (!HaveInsertPoint())
insertPointSet = true;

assert(builder.getInsertionBlock() && "expected an insertion block");
}

/// Whether any type-checking sanitizers are enabled. If \c false, calls to
Expand Down Expand Up @@ -1280,6 +1293,7 @@ class CIRGenFunction : public CIRGenTypeCache {
assert(CGF.OutermostConditional != nullptr);
if (CGF.OutermostConditional == this)
CGF.OutermostConditional = nullptr;
CGF.ensureInsertPoint();
}

/// Returns the insertion point which will be executed prior to each
Expand Down
22 changes: 22 additions & 0 deletions clang/lib/CIR/CodeGen/CIRGenStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Address CIRGenFunction::emitCompoundStmtWithoutScope(const CompoundStmt &S,
}
}

ensureInsertPoint();
return retAlloca;
}

Expand Down Expand Up @@ -100,6 +101,27 @@ mlir::LogicalResult CIRGenFunction::emitStmt(const Stmt *S,
if (mlir::succeeded(emitSimpleStmt(S, useCurrentScope)))
return mlir::success();

// Check if we are generating unreachable code.
if (!HaveInsertPoint()) {
// If so, and the statement doesn't contain a label, then we do not need to
// generate actual code. This is safe because (1) the current point is
// unreachable, so we don't need to execute the code, and (2) we've already
// handled the statements which update internal data structures (like the
// local variable map) which could be used by subsequent statements.
if (!ContainsLabel(S)) {
// Verify that any decl statements were handled as simple, they may be in
// scope of subsequent reachable statements.
assert(!isa<DeclStmt>(*S) && "Unexpected DeclStmt!");
// TODO(cir): PGO->markStmtMaybeUsed(S);
return mlir::success();
}

// LLVM's codegen makes a new code block, but in CIR we reset
// `insertPointSet` to true, since we don't clear insertion points for
// unreachable code.
ensureInsertPoint();
}

if (getContext().getLangOpts().OpenMP &&
getContext().getLangOpts().OpenMPSimd)
assert(0 && "not implemented");
Expand Down
120 changes: 120 additions & 0 deletions clang/test/CIR/CodeGen/cleanup-null-fixups.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#include "std-cxx.h"

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -I%S/../Inputs -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -I%S/../Inputs -fclangir -emit-llvm %s -o %t.ll
// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -I%S/../Inputs -emit-llvm %s -o %t.og.ll
// RUN: FileCheck --check-prefix=OGCG --input-file=%t.og.ll %s

// CIR-LABEL: cir.func {{.*}} @_ZNK1d1eEv
// CIR-NEXT: %[[V0:.*]] = cir.alloca !cir.ptr<!rec_d>, !cir.ptr<!cir.ptr<!rec_d>>, ["this", init]
// CIR-NEXT: %[[V1:.*]] = cir.alloca !rec_a3A3Ac, !cir.ptr<!rec_a3A3Ac>, ["__retval"]
// CIR-NEXT: %[[V2:.*]] = cir.alloca !rec_aj, !cir.ptr<!rec_aj>, ["ao"]
// CIR-NEXT: cir.store %arg0, %[[V0]] : !cir.ptr<!rec_d>, !cir.ptr<!cir.ptr<!rec_d>>
// CIR-NEXT: %[[V3:.*]] = cir.load %[[V0]] : !cir.ptr<!cir.ptr<!rec_d>>, !cir.ptr<!rec_d>
// CIR-NEXT: cir.scope {
// CIR-NEXT: %[[V5:.*]] = cir.alloca !rec_aj, !cir.ptr<!rec_aj>, ["agg.tmp0"]
// CIR-NEXT: %[[V6:.*]] = cir.get_global @an : !cir.ptr<!rec_aj>
// CIR-NEXT: cir.copy %[[V6]] to %[[V5]] : !cir.ptr<!rec_aj>
// CIR-NEXT: %[[V7:.*]] = cir.load align(1) %[[V5]] : !cir.ptr<!rec_aj>, !rec_aj
// CIR-NEXT: cir.call @_ZN1a1cC1I2ajEET_(%[[V1]], %[[V7]]) : (!cir.ptr<!rec_a3A3Ac>, !rec_aj) -> ()
// CIR-NEXT: cir.call @_ZN2ajD1Ev(%[[V5]]) : (!cir.ptr<!rec_aj>) -> ()
// CIR-NEXT: }
// CIR-NEXT: cir.call @_ZN2ajD1Ev(%[[V2]]) : (!cir.ptr<!rec_aj>) -> ()
// CIR-NEXT: %[[V4:.*]] = cir.load align(1) %[[V1]] : !cir.ptr<!rec_a3A3Ac>, !rec_a3A3Ac
// CIR-NEXT: cir.return %[[V4]] : !rec_a3A3Ac
// CIR-NEXT: }

// LLVM-LABEL: {{.*}} @_ZNK1d1eEv(ptr {{.*}})
// LLVM-NEXT: %[[V2:.*]] = alloca %class.aj, i64 1, align 1
// LLVM-NEXT: %[[V3:.*]] = alloca ptr, i64 1, align 8
// LLVM-NEXT: %[[V4:.*]] = alloca %"class.a::c", i64 1, align 1
// LLVM-NEXT: %[[V5:.*]] = alloca %class.aj, i64 1, align 1
// LLVM-NEXT: store ptr {{.*}}, ptr %[[V3]], align 8
// LLVM-NEXT: %[[V6:.*]] = load ptr, ptr %[[V3]], align 8
// LLVM-NEXT: br label %[[B7:.*]]
// LLVM: [[B7]]:
// LLVM-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr %[[V2]], ptr @an, i32 1, i1 false)
// LLVM-NEXT: %[[V8:.*]] = load %class.aj, ptr %[[V2]], align 1
// LLVM-NEXT: call void @_ZN1a1cC1I2ajEET_(ptr %[[V4]], %class.aj %[[V8]])
// LLVM-NEXT: call void @_ZN2ajD1Ev(ptr %[[V2]])
// LLVM-NEXT: br label %[[B9:.*]]
// LLVM: [[B9]]:
// LLVM-NEXT: call void @_ZN2ajD1Ev(ptr %[[V5]])
// LLVM-NEXT: %[[V10:.*]] = load %"class.a::c", ptr %[[V4]], align 1
// LLVM-NEXT: ret %"class.a::c" %[[V10]]
// LLVM-NEXT: }

// OGCG-LABEL: {{.*}} @_ZNK1d1eEv(ptr {{.*}})
// OGCG: %{{.*}} = alloca ptr, align 8
// OGCG-NEXT: %{{.*}} = alloca ptr, align 8
// OGCG-NEXT: %{{.*}} = alloca %{{.*}}, align 1
// OGCG-NEXT: %{{.*}} = alloca %{{.*}}, align 1
// OGCG-NEXT: store ptr %{{.*}}, ptr %{{.*}}, align 8
// OGCG-NEXT: store ptr %{{.*}}, ptr %{{.*}}, align 8
// OGCG-NEXT: %{{.*}} = load ptr, ptr %{{.*}}, align 8
// OGCG-NEXT: call void @_ZN1a1cC1I2ajEET_(ptr noundef nonnull align 1 dereferenceable(1) %{{.*}}, ptr noundef %{{.*}})
// OGCG-NEXT: call void @_ZN2ajD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %{{.*}})
// OGCG-NEXT: call void @_ZN2ajD1Ev(ptr noundef nonnull align 1 dereferenceable(1) %{{.*}})
// OGCG-NEXT: ret void
Copy link
Member

@bcardosolopes bcardosolopes Jan 21, 2026

Choose a reason for hiding this comment

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

Hmmm, back to the trap being omitted here

Something to consider in the future is how to better mimic the OG, since in cases like this the block with the TrapOp is completely erased.

Do you get the llvm.trap if you use -disable-llvm-passes? We need to make sure we understand why it's different before accepting a different outcome.

Copy link
Contributor Author

@bruteforceboy bruteforceboy Jan 22, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We don't get llvm.trap even when we use -disable-llvm-passes, because it is not emitted at all by the OG CodeGen in this. I'll explain my findings in more detail, hopefully without boring you with details.

I did some digging to understand how the OG handles return statements in scopes. For my experiment, I used:

#include <string>

int foo() {
  std::string a;
  return 0; 
  std::string b;
}

This reproduces the same null-fixup crash using the standard library, which was easier to reason about. The AST for the function body looks roughly like:

DeclStmt 0x5ad8b41c0eb8
`-VarDecl 0x5ad8b41c0df8  a 'std::string':'class std::basic_string<char>' callinit destroyed
  `-CXXConstructExpr 0x5ad8b41c0e90 'std::string':'class std::basic_string<char>' 'void (void) noexcept(is_nothrow_default_constructible<std::allocator<char> >::value)'
ReturnStmt 0x5ad8b41c0ef0
`-IntegerLiteral 0x5ad8b41c0ed0 'int' 0
DeclStmt 0x5ad8b41c0fc8
`-VarDecl 0x5ad8b41c0f38  b 'std::string':'class std::basic_string<char>' callinit destroyed
  `-CXXConstructExpr 0x5ad8b41c0fa0 'std::string':'class std::basic_string<char>' 'void (void) noexcept(is_nothrow_default_constructible<std::allocator<char> >::value)'

The OG IR output eventually looks something like:

define dso_local noundef i32 @_Z3foov() #0 {
entry:
  %a = alloca %"class.std::__cxx11::basic_string", align 8
  %b = alloca %"class.std::__cxx11::basic_string", align 8
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEC1Ev(ptr noundef nonnull align 8 dereferenceable(32) %a) #2
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEED1Ev(ptr noundef nonnull align 8 dereferenceable(32) %a) #2
  ret i32 0
}

Notice the ctor & dtor for %b are omitted, and this is with the passes disabled.

The statements in the function are emitted through EmitFunctionBody. The first DeclStmt is emitted just fine, then at the end of CodeGenFunction::EmitReturnStmt for the second statement, we have:

cleanupScope.ForceCleanup();
EmitBranchThroughCleanup(ReturnBlock);

We force the cleanup for the current scope and then emit a branch through the cleanup. In EmitBranchThroughCleanup, we clear the insertion point (Builder.ClearInsertionPoint()) at the end. This signals to the rest of the codegen paths that they should not emit statements further in this scope.

So, when emitting the last DeclStmt through CodeGenFunction::EmitStmt, we reach this in CGDecl from my example:

EmitAutoVarInit(emission);
EmitAutoVarCleanups(emission);

and both EmitAutoVarDecl and EmitAutoVarCleanups do not emit anything if the insertion point isn't set [1], [2].

PS: There are some caveats, such as cases with labels, that insert a trap, but that's not very relevant here.

In CIR, as I understand, our cleanups aren't mature enough [3], [4], [5], [6], [7], and we don't clear insertion points at the moment or properly mimic the unreachable code behavior.

So, with my PR our llvm output for this example looks like:

define dso_local i32 @_Z3foov() #1 {
  %1 = alloca i32, i64 1, align 4
  %2 = alloca %"class.std::__cxx11::basic_string<char>", i64 1, align 8
  %3 = alloca %"class.std::__cxx11::basic_string<char>", i64 1, align 8
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEC1Ev(ptr %2)
  store i32 0, ptr %1, align 4
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEED1Ev(ptr %2)
  %4 = load i32, ptr %1, align 4
  ret i32 %4

5:                                                ; No predecessors!
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEC1Ev(ptr %3)
  call void @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEED1Ev(ptr %3)
  call void @llvm.trap()
  unreachable
}

This is why I added the TrapOp for blocks that are unreachable, as a starting point, and mentioned handling such blocks in the future. As CIR matures, these traps should become redundant without affecting correctness.

Copy link
Member

@bcardosolopes bcardosolopes Jan 23, 2026

Choose a reason for hiding this comment

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

I understand what you are doing now, thanks for attempting a solution. I don't see how this brings us any closer to improve the situation though, fixing issues and making cleanup more mature is the desired direction to go.

Copy link
Contributor Author

@bruteforceboy bruteforceboy Jan 23, 2026

Choose a reason for hiding this comment

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

That makes sense. I will rework this PR in a bit to use the null insertion point logic from the OG, and I'll get rid of the traps.

// OGCG-NEXT: }

inline namespace a {
class c {
public:
template <typename b> c(b);
~c();
};
} // namespace a
class d {
c e() const;
};
class aj {
public:
~aj();
} an;
c d::e() const {
aj ao;
return an;
c(0);
}

// CIR-LABEL: cir.func {{.*}} @_Z3foov
// CIR-NEXT: %[[V0:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] {alignment = 4 : i64}
// CIR-NEXT: %[[V1:.*]] = cir.alloca !rec_std3A3Abasic_string3Cchar3E, !cir.ptr<!rec_std3A3Abasic_string3Cchar3E>, ["a", init]
// CIR-NEXT: %[[V2:.*]] = cir.alloca !rec_std3A3Abasic_string3Cchar3E, !cir.ptr<!rec_std3A3Abasic_string3Cchar3E>, ["b"]
// CIR-NEXT: cir.call @_ZNSbIcEC1Ev(%[[V1]]) : (!cir.ptr<!rec_std3A3Abasic_string3Cchar3E>) -> ()
// CIR-NEXT: %[[V3:.*]] = cir.const #cir.int<0> : !s32i
// CIR-NEXT: cir.store align(4) %[[V3]], %[[V0]] : !s32i, !cir.ptr<!s32i>
// CIR-NEXT: cir.call @_ZNSbIcED1Ev(%[[V1]]) : (!cir.ptr<!rec_std3A3Abasic_string3Cchar3E>) -> () extra(#fn_attr)
// CIR-NEXT: %[[V4:.*]] = cir.load align(4) %[[V0]] : !cir.ptr<!s32i>, !s32i
// CIR-NEXT: cir.return %[[V4]] : !s32i
// CIR-NEXT: }

// LLVM-LABEL: {{.*}} @_Z3foov()
// LLVM-NEXT: %[[V1:.*]] = alloca i32, i64 1, align 4
// LLVM-NEXT: %[[V2:.*]] = alloca %"class.std::basic_string<char>", i64 1, align 1
// LLVM-NEXT: %[[V3:.*]] = alloca %"class.std::basic_string<char>", i64 1, align 1
// LLVM-NEXT: call void @_ZNSbIcEC1Ev(ptr %[[V2]])
// LLVM-NEXT: store i32 0, ptr %[[V1]], align 4
// LLVM-NEXT: call void @_ZNSbIcED1Ev(ptr %[[V2]])
// LLVM-NEXT: %[[V4:.*]] = load i32, ptr %[[V1]], align 4
// LLVM-NEXT: ret i32 %[[V4]]
// LLVM-NEXT: }

// OGCG-LABEL: {{.*}} @_Z3foov()
// OGCG: %[[a:.*]] = alloca %"class.std::basic_string", align 1
// OGCG-NEXT: %[[b:.*]] = alloca %"class.std::basic_string", align 1
// OGCG-NEXT: call void @_ZNSbIcEC1Ev(ptr noundef nonnull align 1 dereferenceable(1) %[[a]])
// OGCG-NEXT: call void @_ZNSbIcED1Ev(ptr noundef nonnull align 1 dereferenceable(1) %[[a]])
// OGCG-NEXT: ret i32 0
// OGCG-NEXT: }

int foo() {
std::string a;
return 0;
std::string b;
}
31 changes: 0 additions & 31 deletions clang/test/CIR/crashes/cleanup-892-null-fixups.cpp
View file
Open in desktop

This file was deleted.