[PATCH] D77124: Handle CET for -exception-model sjlj

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm marked 2 inline comments as done.
xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp:145
+        else
+          Changed |= addENDBR(MBB, I);
+        EHPadIBTNeeded = false;
----------------
LuoYuanke wrote:
> 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?
> Do we need to insert endbr on each EHPad?
Yes
Two EHPad must be in different MBB, right?
Right

I'll update the test case, let it easy to see. Thank you!


================
Comment at: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp:149
+      if (!IsEHPadBB && I->isEHLabel() && EHPadIBTNeeded) {
+        MCSymbol *Sym = I->getOperand(0).getMCSymbol();
+        if (MF.hasCallSiteLandingPad(Sym))
----------------
LuoYuanke wrote:
> 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
> ```
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.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77124/new/

https://reviews.llvm.org/D77124