In SjLj exception mode, the old landingpad BB will create a new landingpad BB and use indirect branch jump to the old landingpad BB in lowering.
So we should add 2 endbr for this exception model.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
172 | Can you comments with an example to elaborate which pointer endbr is needed? Do we need to insert endbr on each EHPad? Two EHPad must be in different MBB, right? |
llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll | ||
---|---|---|
69 | The test just show an endbr is inserted, but it doesn't demo where the endbr is inserted. |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
176 | After build a local test case, I can see in line 39 it has indirect jump to lpad. Can you explain why checking "MF.hasCallSiteLandingPad(Sym)" is needed? Is there any test case for it? 37 .LBB0_7: # in Loop: Header=BB0_6 Depth=1 38 leaq .LJTI0_0(%rip), %rcx 39 jmpq *(%rcx,%rax,8) 40 .LBB0_1: # %lpad 41 # in Loop: Header=BB0_6 Depth=1 42 .Ltmp2: 43 movl -132(%rbp), %edi 44 movl -128(%rbp), %eax |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
172 |
Yes I'll update the test case, let it easy to see. Thank you! | |
176 | Because before converting to SjLj exception mode, this information "MF.hasCallSiteLandingPad(Sym) is true" has been established. And it do not changed after converting to SjLj exception mode. in fact, line 39 jump to line 42(not line 40), .Ltmp2 is a hasCallSiteLandingPad symbol. |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
151 | Why do you have to iterate the same loop twice? |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
151 | I move out the condition out of the loop, because in most time the "if" and "else if" is false. | |
llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll | ||
69 | Yes, here just shows there one more endbr in sjlj model than other model. I'll refine it. |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
152 | The isEHPad() include CatchSwitch, CatchPad, CleanupPad and LandingPad. Do we want to insert endbr to all of them or just for LandingPad? |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
152 | The LandingPad is the exception where will be indirectly jump when the exception happen. The others uses direct jump, them belong to the "internal exception handle block" following the LandingPad. |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
152 | Yes. Could you extend your test case to cover CatchPad or some other Pad to make sure we don't insert endbr on these blocks. |
llvm/lib/Target/X86/X86IndirectBranchTracking.cpp | ||
---|---|---|
152 | OK, I'll use CHECK-COUNT =n to limit the count of endbr in the tests. |
Update test about endbr checking for sjlj model.
And from the test you can see the endbr is not add before catchpad, total 3 endbr, one at the entry of function, 2nd one before "old landpad" (line62) and last one at the begin of BB (new landpad without name) generated before "old landpad"(line 51)
llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll | ||
---|---|---|
80 | Also add a cache pad. Some code as this. catch: ; preds = %lpad %5 = tail call i8* @__cxa_begin_catch(i8* %2) #6 %6 = bitcast i8* %5 to i32* |
Add a test to enhance the patch.
llvm/test/CodeGen/X86/indirect-branch-tracking-eh.ll | ||
---|---|---|
80 | OK, but this will make test a little complex, in fact the patch is focus on dealing with the change of landpad (old +new). So current test is written very simple to show this. |
Why do you have to iterate the same loop twice?