[llvm-commits] patch: use addressing modes for "m" constraints fed by bitcast/GEP

[Nick Lewycky]

On 30 September 2011 17:20, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Fri, Sep 30, 2011 at 5:05 PM, Nick Lewycky <nlewycky at google.com> wrote:
> > On 30 September 2011 16:30, Eli Friedman <eli.friedman at gmail.com> wrote:
> >>
> >> On Fri, Sep 30, 2011 at 3:26 PM, Nick Lewycky <nlewycky at google.com>
> wrote:
> >> > The attached patch eagerly evaluates bitcasts and GEPs feeding inline
> >> > assembly constraints. That reduces the chance that we'll copy the
> >> > result into a register. On this testcase:
> >> >
> >> > void test(short *a,short *b,float *c) {
> >> >    char tmp[256];
> >> >    __asm__ volatile(
> >> >        "%0 %1 %2 %3 %4"
> >> >        :
> >> >        :"m"(a),"m"(b),"m"(c),"m"(tmp[0]), "m"(tmp[128])
> >> >        :"memory","%eax","%ebx","%ecx","%edx","%esi","%edi");
> >> > }
> >> >
> >> > previously we'd run out of registers. Now, we emit:
> >> >
> >> >        pushl   %ebx
> >> >        pushl   %edi
> >> >        pushl   %esi
> >> >        subl    $268, %esp              # imm = 0x10C
> >> >        movl    292(%esp), %eax
> >> >        movl    288(%esp), %ecx
> >> >        movl    284(%esp), %edx
> >> >        movl    %edx, 264(%esp)
> >> >        movl    %ecx, 260(%esp)
> >> >        movl    %eax, 256(%esp)
> >> >        #APP
> >> >        264(%esp) 260(%esp) 256(%esp) (%esp) 128(%esp)
> >> >        #NO_APP
> >> >        addl    $268, %esp              # imm = 0x10C
> >> >        popl    %esi
> >> >        popl    %edi
> >> >        popl    %ebx
> >> >        ret
> >> >
> >> > which clearly has an extra copy, but that's a bug which existed
> >> > without my patch anyways. The tmp[0] and tmp[128] don't have any
> >> > copies.
> >> >
> >> > Please review!
> >>
> >> I follow the concept, but I don't think the place you're hacking it in
> >> is really appropriate.  This is a fast-isel-specific problem, so it
> >> seems like the fix should also be in fast-isel code.
> >
> > Thanks. I don't like where it is now, but I don't see where else to put
> > it. It is not fast-isel specific, the same testcase fails at -O2 before
> and
> > works after. Can't explain that!
>
> The given testcase works fine for me at -O2 without your patch... is
> there some case which doesn't work?
>

You're right. I fooled myself at some point by producing a testcase that
failed under -O2, but then that testcase failed under GCC anyways.

I'll see if I can move this into FastISel specific code path, somewhere.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110930/7cb32061/attachment.html>