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

On Sat, Oct 03, 2015 at 08:43:49AM -0700, Tim Northover
wrote:
> Hi Joerg,
> 
> 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 we should view this as a special case of bitcasting functions
> to different types (not quite types because it turns out function
> attributes aren't part of the type in IR, but morally the same thing).
> LLVM allows those casts normally.
Well, we do consider it an ABI changing attribute for the purpose of
tail call (under "musttail") and we are loosing the dereferencable
information. Removing the assert is easy, I just want to make sure there
is consensus it is the right thing to do.

Joerg

On Sat, Oct 3, 2015 at 2:46 PM, Joerg Sonnenberger via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Sat, Oct 03, 2015 at 08:43:49AM -0700, Tim Northover wrote:
> > Hi Joerg,
> >
> > 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 we should view this as a special case of bitcasting functions
> > to different types (not quite types because it turns out function
> > attributes aren't part of the type in IR, but morally the same
thing).
> > LLVM allows those casts normally.
>
> Well, we do consider it an ABI changing attribute for the purpose of
> tail call (under "musttail") and we are loosing the
dereferencable
> information. Removing the assert is easy, I just want to make sure there
> is consensus it is the right thing to do.
>
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.

This seems like one of those things that's supposed to be in the
"lint"
pass, but isn't, because lint isn't part of the normal clang pass
pipeline.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151005/d12306ad/attachment.html>

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