Skip to content

Commit c245e6f

Browse files
bruteforceboylanza
bruteforceboy
authored and
lanza
committed
[CIR][CodeGen] Emit cir.resume for synthetic TryOp's in place (#1418)
Currently, the following code snippet fails during CIR codegen with exceptions enabled: ``` #include <string> void foo(const char *path) { std::string str = path; str = path; str = path; } ``` using `bin/clang++ tmp.cpp -fclangir -Xclang -emit-cir -S`, the error: ``` error: empty block: expect at least a terminator ``` Relevant part of the CIR before verification looks like: ``` %118 = "cir.load"(%114) : (!cir.ptr<!cir.ptr<!cir.int<s, 8>>>) -> !cir.ptr<!cir.int<s, 8>> "cir.try"() <{catch_types = [#cir.unwind], cleanup, synthetic}> ({ %123 = "cir.call"(%115, %118) <{ast = #cir.call.expr.ast, callee = @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEaSEPKc, calling_conv = 1 : i32, exception, extra_attrs = #cir<extra({})>, side_effect = 1 : i32}> ({ "cir.call"(%115) <{callee = @_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEED1Ev, calling_conv = 1 : i32, extra_attrs = #cir<extra({nothrow = #cir.nothrow})>, side_effect = 1 : i32}> ({ }) : (!cir.ptr<!cir.struct<class "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>" {!cir.struct<struct "std::__cxx11::basic_string<char>::_Alloc_hider" {!cir.ptr<!cir.int<s, 8>>} #cir.record.decl.ast>, !cir.int<u, 64>, !cir.struct<union "anon.0" padded {!cir.array<!cir.int<s, 8> x 16>, !cir.int<u, 64>, !cir.array<!cir.int<u, 8> x 8>} #cir.record.decl.ast>} #cir.record.decl.ast>>) -> () "cir.yield"() : () -> () }) : (!cir.ptr<!cir.struct<class "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>" {!cir.struct<struct "std::__cxx11::basic_string<char>::_Alloc_hider" {!cir.ptr<!cir.int<s, 8>>} #cir.record.decl.ast>, !cir.int<u, 64>, !cir.struct<union "anon.0" padded {!cir.array<!cir.int<s, 8> x 16>, !cir.int<u, 64>, !cir.array<!cir.int<u, 8> x 8>} #cir.record.decl.ast>} #cir.record.decl.ast>>, !cir.ptr<!cir.int<s, 8>>) -> !cir.ptr<!cir.struct<class "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>" {!cir.struct<struct "std::__cxx11::basic_string<char>::_Alloc_hider" {!cir.ptr<!cir.int<s, 8>>} #cir.record.decl.ast>, !cir.int<u, 64>, !cir.struct<union "anon.0" padded {!cir.array<!cir.int<s, 8> x 16>, !cir.int<u, 64>, !cir.array<!cir.int<u, 8> x 8>} #cir.record.decl.ast>} #cir.record.decl.ast>> "cir.store"(%123, %116) : (!cir.ptr<!cir.struct<class "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>" {!cir.struct<struct "std::__cxx11::basic_string<char>::_Alloc_hider" {!cir.ptr<!cir.int<s, 8>>} #cir.record.decl.ast>, !cir.int<u, 64>, !cir.struct<union "anon.0" padded {!cir.array<!cir.int<s, 8> x 16>, !cir.int<u, 64>, !cir.array<!cir.int<u, 8> x 8>} #cir.record.decl.ast>} #cir.record.decl.ast>>, !cir.ptr<!cir.ptr<!cir.struct<class "std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>" {!cir.struct<struct "std::__cxx11::basic_string<char>::_Alloc_hider" {!cir.ptr<!cir.int<s, 8>>} #cir.record.decl.ast>, !cir.int<u, 64>, !cir.struct<union "anon.0" padded {!cir.array<!cir.int<s, 8> x 16>, !cir.int<u, 64>, !cir.array<!cir.int<u, 8> x 8>} #cir.record.decl.ast>} #cir.record.decl.ast>>>) -> () "cir.yield"() : () -> () }, { ^bb0: <--- EMPTY BLOCK }) : () -> () ``` There is an empty block! If you extend the snippet with more `str = path;`, you get more empty blocks... The issue is the `cir.resume` ops which should be in those empty blocks from synthetic TryOp's aren't linked properly during the cleanup. My suggestion: We should explicitly add `cir.resume` for synthetic tryOp's, because we already know they have just an [unwind handler](https://github.com/llvm/clangir/blob/8746bd4bbe777352c2935e9937449637a8943767/clang/lib/CIR/CodeGen/CIRGenCall.cpp#L506). So, during [CIRGenCleanup](https://github.com/llvm/clangir/blob/8746bd4bbe777352c2935e9937449637a8943767/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp#L667) we don't need to add `cir.resume` for synthetic TryOp's. This PR adds this and a test.
1 parent a844b1b commit c245e6f

File tree

3 files changed

+96
-4
lines changed

3 files changed

+96
-4
lines changed

‎clang/lib/CIR/CodeGen/CIRGenCall.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ emitCallLikeOp(CIRGenFunction &CGF, mlir::Location callLoc,
507507
// handler: unwind.
508508
auto *r = result.addRegion();
509509
builder.createBlock(r);
510+
builder.create<cir::ResumeOp>(loc, mlir::Value{}, mlir::Value{});
510511
});
511512
op.setSynthetic(true);
512513
return op;

‎clang/lib/CIR/CodeGen/CIRGenCleanup.cpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -666,10 +666,12 @@ void CIRGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
666666
if (nextAction == ehResumeBlock) {
667667
if (auto tryToPatch =
668668
currYield->getParentOp()->getParentOfType<cir::TryOp>()) {
669-
mlir::Block *resumeBlockToPatch =
670-
tryToPatch.getCatchUnwindEntryBlock();
671-
emitEHResumeBlock(/*isCleanup=*/true, resumeBlockToPatch,
672-
tryToPatch.getLoc());
669+
if (!tryToPatch.getSynthetic()) {
670+
mlir::Block *resumeBlockToPatch =
671+
tryToPatch.getCatchUnwindEntryBlock();
672+
emitEHResumeBlock(/*isCleanup=*/true, resumeBlockToPatch,
673+
tryToPatch.getLoc());
674+
}
673675
}
674676
}
675677

‎clang/test/CIR/CodeGen/synthetic-try-resume.cpp

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -I%S/../Inputs -fclangir -emit-cir %s -o %t.cir
2+
// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
3+
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcxx-exceptions -fexceptions -I%S/../Inputs -fclangir -emit-llvm %s -o %t.ll
4+
// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s
5+
6+
#include "std-cxx.h"
7+
8+
// CIR-LABEL: @_Z3fooPKc
9+
// LLVM-LABEL: @_Z3fooPKc
10+
11+
void foo(const char* path) {
12+
std::string str = path;
13+
str = path;
14+
str = path;
15+
}
16+
17+
// CIR: cir.try synthetic cleanup {
18+
// CIR: cir.call exception @_ZNSbIcEC1EPKcRKNS_9AllocatorE({{.*}}, {{.*}}, {{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!s8i>, !cir.ptr<!ty_std3A3Abasic_string3Cchar3E3A3AAllocator>) -> () cleanup {
19+
// CIR: cir.call @_ZNSbIcED1Ev({{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> ()
20+
// CIR: cir.yield
21+
// CIR: }
22+
// CIR: cir.yield
23+
// CIR: } catch [#cir.unwind {
24+
// CIR: cir.resume
25+
// CIR: }]
26+
// CIR: cir.try synthetic cleanup {
27+
// CIR: {{.*}} = cir.call exception @_ZNSbIcEaSERKS_({{.*}}, {{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> !cir.ptr<!ty_std3A3Abasic_string3Cchar3E> cleanup {
28+
// CIR: cir.call @_ZNSbIcED1Ev({{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> ()
29+
// CIR: cir.yield
30+
// CIR: }
31+
// CIR: cir.store {{.*}}, {{.*}} : !cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>>
32+
// CIR: cir.yield
33+
// CIR: } catch [#cir.unwind {
34+
// CIR: cir.resume
35+
// CIR: }]
36+
// CIR: {{.*}} = cir.load {{.*}} : !cir.ptr<!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>>, !cir.ptr<!ty_std3A3Abasic_string3Cchar3E>
37+
// CIR: cir.call @_ZNSbIcED1Ev({{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> ()
38+
// CIR: cir.try synthetic cleanup {
39+
// CIR: cir.call exception @_ZNSbIcEC1EPKcRKNS_9AllocatorE({{.*}}, {{.*}}, {{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!s8i>, !cir.ptr<!ty_std3A3Abasic_string3Cchar3E3A3AAllocator>) -> ()
40+
// CIR: cir.yield
41+
// CIR: } catch [#cir.unwind {
42+
// CIR: cir.resume
43+
// CIR: }]
44+
// CIR: cir.try synthetic cleanup {
45+
// CIR: {{.*}} = cir.call exception @_ZNSbIcEaSERKS_({{.*}}, {{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> !cir.ptr<!ty_std3A3Abasic_string3Cchar3E> cleanup {
46+
// CIR: cir.call @_ZNSbIcED1Ev({{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> ()
47+
// CIR: cir.call @_ZNSbIcED1Ev({{.*}}) : (!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>) -> ()
48+
// CIR: cir.yield
49+
// CIR: }
50+
// CIR: cir.store {{.*}}, {{.*}} : !cir.ptr<!ty_std3A3Abasic_string3Cchar3E>, !cir.ptr<!cir.ptr<!ty_std3A3Abasic_string3Cchar3E>>
51+
// CIR: cir.yield
52+
// CIR: } catch [#cir.unwind {
53+
// CIR: cir.resume
54+
// CIR: }]
55+
56+
// LLVM: invoke void @_ZNSbIcEC1EPKcRKNS_9AllocatorE(ptr {{.*}}, ptr {{.*}}, ptr {{.*}})
57+
// LLVM: to label {{.*}} unwind label %[[B18:.*]]
58+
// LLVM: [[B18]]
59+
// LLVM: call void @_ZNSbIcED1Ev(ptr {{.*}})
60+
// LLVM: br label %[[B22:.*]]
61+
// LLVM: [[B22]]
62+
// LLVM: resume { ptr, i32 } {{.*}}
63+
// LLVM: {{.*}}:
64+
// LLVM: {{.*}} = invoke ptr @_ZNSbIcEaSERKS_(ptr {{.*}}, ptr {{.*}})
65+
// LLVM: to label {{.*}} unwind label %[[B31:.*]]
66+
// LLVM: [[B31]]
67+
// LLVM: call void @_ZNSbIcED1Ev(ptr {{.*}})
68+
// LLVM: br label %[[B35:.*]]
69+
// LLVM: [[B35]]
70+
// LLVM: resume { ptr, i32 } {{.*}}
71+
// LLVM: {{.*}}:
72+
// LLVM: call void @_ZNSbIcED1Ev(ptr {{.*}})
73+
// LLVM: br label {{.*}}
74+
// LLVM: {{.*}}:
75+
// LLVM: invoke void @_ZNSbIcEC1EPKcRKNS_9AllocatorE(ptr {{.*}}, ptr {{.*}}, ptr {{.*}})
76+
// LLVM: to label {{.*}} unwind label %[[B46:.*]]
77+
// LLVM: [[B46]]
78+
// LLVM: br label %[[B50:.*]]
79+
// LLVM: [[B50]]
80+
// LLVM: resume { ptr, i32 } {{.*}}
81+
// LLVM: {{.*}}:
82+
// LLVM: {{.*}} = invoke ptr @_ZNSbIcEaSERKS_(ptr {{.*}}, ptr {{.*}})
83+
// LLVM: to label {{.*}} unwind label %[[B59:.*]]
84+
// LLVM: [[B59]]
85+
// LLVM: call void @_ZNSbIcED1Ev(ptr {{.*}})
86+
// LLVM: call void @_ZNSbIcED1Ev(ptr {{.*}})
87+
// LLVM: br label %[[B63:.*]]
88+
// LLVM: [[B63]]
89+
// LLVM: resume { ptr, i32 } {{.*}}

0 commit comments

Comments
 (0)