pengfei added inline comments.
================
Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:346
+ auto IsIndirectTailCall = [I, &Desc]() {
+ return Desc.isCall() && Desc.isReturn() && Desc.isBarrier() &&
+ !I->getOperand(0).isGlobal();
----------------
nickdesaulniers wrote:
> Why `isBarrier`? Maybe add a comment that mentions a specific TableGen record
> from llvm/lib/Target/X86/X86InstrControl.td? That would help me verify that
> `isBarrier()` is correct here.
>
> I'm guessing this is:
>
> 363 let isCall = 1, isTerminator = 1, isReturn = 1, isBarrier = 1,
>
> 364 isCodeGenOnly = 1, Uses = [RSP, SSP] in {
>
> block in llvm/lib/Target/X86/X86InstrControl.td.
>
> Otherwise llvm/lib/Target/X86/X86FrameLowering.cpp has a static function that
> I think is precisely what we want here, called ` isTailCallOpcode`. Perhaps
> move that to a header shared between the two translation units then reuse it
> here? A few other backends have an `IsTailCall` method though they're
> inconsistent where they define it. It still would be good though to try to
> match existing conventions which makes jumping between backends easier.
> Why `isBarrier`?
Because we have conditional tail calls, see defination of `TCRETURNdicc`.
> Perhaps move that to a header shared between the two translation units then
> reuse it here?
We can't. They are not overlapped.
> A few other backends have an `IsTailCall` method though they're inconsistent
> where they define it
`IsTailCall` is an attribute of call instruction in IR. We lost it in MIR phase.
================
Comment at: llvm/lib/Target/X86/X86AsmPrinter.cpp:347
+ return Desc.isCall() && Desc.isReturn() && Desc.isBarrier() &&
+ !I->getOperand(0).isGlobal();
+ };
----------------
nickdesaulniers wrote:
> Rather than `!...isGlobal()`, would it be more precise to use
> `getOperand(0).isReg()`? Looking at all of the values of `enum
> MachineOperandType`, I think it would be more precise if we check the operand
> is of one type, rather than "not one type."
OK, check opcode instead.
================
Comment at: llvm/test/CodeGen/X86/speculation-hardening-sls.ll:7
+; CHECK: jle
+; CEHCK-NOT: int3
+; CHECK: retq
----------------
nickdesaulniers wrote:
> typo
Good catch!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126137/new/
https://reviews.llvm.org/D126137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
