[PATCH] D105807: [X86] pr51000 in-register struct return tailcalling

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan marked 3 inline comments as done.
urnathan added inline comments.


================
Comment at: llvm/test/CodeGen/X86/sibcall.ll:1026-1027
-; X86-NEXT:    movl %eax, (%esp)
-; X86-NEXT:    calll t22_f_sret at PLT
-; X86-NEXT:    addl $8, %esp
-; X86-NEXT:    retl
----------------
rnk wrote:
> Hang on, this seems wrong. The stack adjustments aren't balanced.
> 
> I think we can't do TCO for 32-bit x86. It pops off the sret pointer. See this IR:
> ```
> define ccc void @t22_f_sret(i32* sret(i32) %p) {
>         store i32 0, i32* %p
>         ret void
> }
> define ccc void @t22_non_sret_to_sret(i32* %agg.result) nounwind  {
>         tail call ccc void @t22_f_sret(i32* noalias sret(i32) %agg.result) nounwind
>         ret void
> }
> ```
> ->
> $ llc t.ll -o - -mtriple=i686-linux-gnu
> ```
> t22_f_sret:                             # @t22_f_sret
>         .cfi_startproc
> # %bb.0:
>         movl    4(%esp), %eax
>         movl    $0, (%eax)
>         retl    $4
> .Lfunc_end0:
> ...
> t22_non_sret_to_sret:                   # @t22_non_sret_to_sret
> # %bb.0:
>         subl    $12, %esp
>         movl    16(%esp), %eax
>         movl    %eax, (%esp)
>         calll   t22_f_sret at PLT
>         addl    $8, %esp
>         retl
> ```
> 
> The `retl $4` instruction here is key, it adjusts ESP to pop off extra argument memory.
oh, I didn't realize sret was callee pop in that case.  There is later code in the tailcall checking, concerning caller and callee pop matching, but it doesn't seem prepared to deal with partial popping.  Simpler just to explicitly check for i686.


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

https://reviews.llvm.org/D105807