[PATCH] D106000: [NFC] X86: Annotate sibcall test

Wed Jul 14 14:59:10 PDT 2021

urnathan added inline comments.

================
Comment at: llvm/test/CodeGen/X86/sibcall.ll:252
+; QOI: We don't sibcall this for unknown reasons (the unreachable
+; rather than a ret?)
 define dso_local void @t10() nounwind ssp {
----------------
mizvekov wrote:
> I think this one has to do with this: https://github.com/llvm/llvm-project/blob/3bf101f34cd466f103af00c764dc1cddb6eb14a6/llvm/lib/CodeGen/Analysis.cpp#L506
> 
> ```
> The block must end in a return statement or unreachable.
> 
> FIXME: Decline tailcall if it's not guaranteed and if the block ends in
> an unreachable, for now. The way tailcall optimization is currently
> implemented means it will add an epilogue followed by a jump. That is
> not profitable. Also, if the callee is a special function (e.g.
> longjmp on x86), it can end up causing miscompilation that has not
> been fully understood.
> ```
> 
> The tail call is not done just because it is not a musttail and the code generated might have worse runtime cost.
> 
> So when we say we have a QOI issue, there are two issues conflated here:
> * Do we know how to do it profitably purely from a instruction cost perspective?
> * Can we figure out the sequence of instructions we actually need to tail call this at all?
QOI is always a judgement call.  What are you suggesting for this patch?

================
Comment at: llvm/test/CodeGen/X86/sibcall.ll:9
 define dso_local void @t1(i32 %x) nounwind ssp {
+  tail call void @foo() nounwind
+  ret void
----------------
xbolva00 wrote:
> Looks like it was not regenerated by update_llc_test_checks.py.
That seems to be breaking the llvm ir apart around the lit patterns, making them hard to read.  What is the rationale for doing that?

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

https://reviews.llvm.org/D106000