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

[Matheus Izvekov via Phabricator via llvm-commits]

mizvekov 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 {
----------------
urnathan wrote:
> 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?
If we are digging down into the reasons why the sibcall is not performed, even if we do not fully understand why, my suggestion would be to mention if there is a difference in the behavior between musttail or not.

Unless there is other test cases in the suite which already go over this.


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

https://reviews.llvm.org/D106000