llvm dev - Oct 2015 - SRET consistency between declaration and call site

On Mon, Oct 05, 2015 at 03:45:29PM -0700, Reid Kleckner
wrote:
> > > On 3 October 2015 at 02:29, Joerg Sonnenberger via llvm-dev
> > > <llvm-dev at lists.llvm.org> wrote:
> > > > while debugging assertions when building libm for 32bit
Sparc, I hit
> > the
> > > > following IR:
> > > >
> > > >   complex_mul_libcall:
> > > >     call void @__muldc3({ double, double }* sret %tmp,
double %conv,
> > double 0.000000e+00, double %a.real, double %a.imag) #2
> > > >
> > > >   ...
> > > >
> > > >   declare void @__muldc3({ double, double }*, double,
double, double,
> > double)
> > > >
> > > > The same IR is essentially generated for i386 too, so it is
not Sparc
> > > > specific. Unlike i386, Sparc has an assertion in its codegen
that the
> > > > function called must have the sret attribute on
corresponding argument.
> > >
> I think the assertion is probably overzealous and would fire when it
> shouldn't in an LTO setting, but it sounds like it also caught a real
clang
> bug.
Can you give an example of where it would trigger in LTO and when should
not?

Joerg

On Tue, Oct 6, 2015 at 1:21 PM, Joerg Sonnenberger via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Can you give an example of where it would trigger in LTO and when should
> not?

You could imagine that __muldc3 might be implemented in C, and it might be
implemented without using _Complex so that it can be compiled by a compiler
without _Complex support. Instead of using a _Complex double return type,
it would use a pointer outparam as the first parameter, and it would return
that pointer as usual in RAX. Yes, this is a prototype mismatch and
probably UB according to C, but this might be part of an implementation
which knows something about the platform lowering.

Generally we try to make these sorts of things "work". For example, if
you
pass parameters to a function pointer call and we later discover the
function takes no parameters, instcombine will discard the arguments rather
than making the call unreachable. IMO the same logic applies to ABI
attributes like sret.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151006/849a78c6/attachment.html>

On Tue, Oct 06, 2015 at 01:33:17PM -0700, Reid Kleckner
wrote:
> On Tue, Oct 6, 2015 at 1:21 PM, Joerg Sonnenberger via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> 
> > Can you give an example of where it would trigger in LTO and when
should
> > not?
> 
> 
> You could imagine that __muldc3 might be implemented in C, and it might be
> implemented without using _Complex so that it can be compiled by a compiler
> without _Complex support. Instead of using a _Complex double return type,
> it would use a pointer outparam as the first parameter, and it would return
> that pointer as usual in RAX. Yes, this is a prototype mismatch and
> probably UB according to C, but this might be part of an implementation
> which knows something about the platform lowering.
This argument is really not convincing to me. If you have LLVM IR, you
are starting from a frontend that should be supporting complex already.
This really sounds like a case that should be flagged to fix up legacy
code and not something to silently hide. Does code like this (manually
turning structure returns into explicit pointers) actually exist? Note
that the normal way for doing the complex thing is to define a two
element structure, which would still give the sret.

Joerg

On Oct 6, 2015, at 4:33 PM, Reid Kleckner via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> On Tue, Oct 6, 2015 at 1:21 PM, Joerg Sonnenberger via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
> Can you give an example of where it would trigger in LTO and when should
> not?
> 
> You could imagine that __muldc3 might be implemented in C, and it might be
implemented without using _Complex so that it can be compiled by a compiler
without _Complex support. Instead of using a _Complex double return type, it
would use a pointer outparam as the first parameter, and it would return that
pointer as usual in RAX. Yes, this is a prototype mismatch and probably UB
according to C, but this might be part of an implementation which knows
something about the platform lowering.
> 
> Generally we try to make these sorts of things "work". For
example, if you pass parameters to a function pointer call and we later discover
the function takes no parameters, instcombine will discard the arguments rather
than making the call unreachable. IMO the same logic applies to ABI attributes
like sret.
Certainly a mismatch between sret and not-sret from caller to callee could not
possibly work on sparc, because sparc's calling convention for struct return
is totally bonkers.

If you're doing an sret return, you need to return to the call address +12,
instead of +8, because the instruction at the usual caller+8 return address is
expected to be a trap instruction.


Said trap also has the size of the struct encoded in it, and is there in order
that it can be inspected by the callee to verify that the size of the struct the
caller allocated is the same as what the callee expected. (Most impls don't
bother with said verification step, because, really, wtf is the point.)

So if the caller thinks it's calling a non-sret function, but it is actually
calling an sret function, it'll put code at +8, and the callee will skip
that. Conversely, if the caller thinks it's calling an sret function, and
the function is not actually, the function will return into the trap at +8. So,
sure, llvm doesn't actually need to detect this mismatch, but the generated
code is guaranteed to be busted if there is such a mismatch, so really nobody
should ever be doing that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151006/b498424f/attachment.html>