[llvm-commits] [llvm] r162821 - /llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cpp

[Jakob Stoklund Olesen]

On Aug 28, 2012, at 6:58 PM, Andrew Trick <atrick at apple.com> wrote:

> Author: atrick
> Date: Tue Aug 28 20:58:55 2012
> New Revision: 162821
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=162821&view=rev
> Log:
> Fix ARM vector copies of overlapping register tuples.

Nice!

> I have tested the fix, but have not been successfull in generating
> a robust unit test. This can only be exposed through particular
> register assignments.

You may be able to build a register labyrinth out of two inline asms, similar to test/CodeGen/ARM/subreg-remat.ll

  %v = vld3
  inline asm, clobbers d8-d31
  inline asm, clobbers d0-d1, d10-d31
  vst3 %v

If you're lucky, RA will split the %v live range between the two inline asms to avoid a spill.

> +  if (!Opc)
> +    llvm_unreachable("Impossible reg-to-reg copy");


Assert? (llvm_unreachable compiles away in -Asserts builds).

> +  // Copy register tuples backward when the first Dest reg overlaps with SrcReg.
> +  if (TRI->regsOverlap(SrcReg, TRI->getSubReg(DestReg, BeginIdx))) {
> +    BeginIdx = BeginIdx + ((SubRegs-1)*Spacing);
> +    Spacing = -Spacing;
> +  }

Should Spacing be signed? Or should there be a comment that modulo arithmetic works?

> +#ifndef NDEBUG
> +  SmallSet<unsigned, 4> DstRegs;
> +#endif
>   for (unsigned i = 0; i != SubRegs; ++i) {
>     unsigned Dst = TRI->getSubReg(DestReg, BeginIdx + i*Spacing);
>     unsigned Src = TRI->getSubReg(SrcReg,  BeginIdx + i*Spacing);
>     assert(Dst && Src && "Bad sub-register");
> +#ifndef NDEBUG
> +    DstRegs.insert(Dst);
> +    assert(!DstRegs.count(Src) && "destructive vector copy");

Maybe swap these two? We do filter identity copies, but it's not an error to ask for one.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120828/348ad0bd/attachment.html>