[llvm-dev] structure-return tailcall

[Nathan Sidwell via llvm-dev]

On 8/19/21 5:14 PM, Reid Kleckner wrote:
> On Wed, Aug 18, 2021 at 11:44 AM Nathan Sidwell via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> 
>     I'm working on pr51000, and thinking about the case of large structures
>     returned by artificial sret pointer parm.
> 
>     I have questions.
> 
>     The itanium ABI requires functions that return a large struct this way,
>     to *also* return that pointer as their scalar return value.  (Let's not
>     get into the pros and cons of that, it is what it is.  I'm looking at
>     x86_64 primarily, but I understand ISAs have similar ABIs.)
> 
> 
> Super nit: it's the psABI that controls how large objects are passed, 
> the Itanium (C++) ABI only cares about the C++-y aspects of a struct. 
> Apologies for the pedantry.

Yeah, I realized that shortly after sending -- I do know these things, 
honest :)

> 
>     However, the X86 sibcall test (llvm/test/CodeGen/X86/sibcall.ll) seems
>     to break that.  Specifically:
> 
>     define fastcc void @t21_sret_to_sret_alloca(%struct.foo* noalias
>     sret(%struct.foo) %agg.result) nounwind  {
>         %a = alloca %struct.foo, align 8
>         tail call fastcc void @t21_f_sret(%struct.foo* noalias
>     sret(%struct.foo) %a) nounwind
>         ret void
>     }
> 
>     That call to t21_f_sret is referencing the frame-allocated %a object.
> 
>     Question: Is sibcall.ll correct or not?
> 
> 
> I think you are correct: the IR will exhibit UB.
> 
> But, that doesn't mean the test case isn't useful. I haven't looked 
> further, but maybe the test is meant to illustrate what would happen if 
> a user did the wrong thing by adding the tail marker when they shouldn't 
> have.

Thanks for confirming my suspicion, I'll add a comment there, when I 
next touch that testcase. Testing for UB seems misguided IMHO (unless 
you're testing for a diagnostic telling you *UB ALERT*).  My concern was 
what if I change the behavior of that particular case in fixing 51000.

>     Implementation/documentation mismatch: I also note that the tail marker
>     can appear even when the call is NOT the last (real) instruction in the
>     function.  That seems strange.
> 
> 
> This is true. The tail marker doesn't really mark call sites in tail 
> positions, it's a statement about aliasing. It simply marks call sites 
> that do not reference stack objects from the current frame. If the call 
> happens to be in the tail position later during codegen, it can become a 
> TCO candidate.

Thanks.  Perhaps I'll be enthused to clarify the documentation.

>     Anyway, the backend code-generator checks additional constraints before
>     performing the tailcall.
> 
>     a) Should the x86 backend track where it assigned the incomming sret
>     pointer and see if that's being passed to the tail call?  (I've not
>     figured out how to do that yet).

> 
> This feels like a target-specific constraint, so I feel like #a is 
> better. You could peek at the IR to make this easy, though.

This is indeed the way I am going -- learning all the DAG-related 
structures.  It seems productive.

thank you for your comments.

nathan

-- 
Nathan Sidwell