[PATCH] Improve performance of vector code on A15

[Silviu Baranga]

Hi Jakob,

Thanks for the review. I'm attaching a new version of the patch that
should have all the comments addressed (unless I've missed
something).

> +  // The only way we can do a partial register update is through a
> COPY,
> +  // INSERT_SUBREG or REG_SEQUENCE.
> +  if (MI->isCopy() && usesRegClass(MI->getOperand(1),
> &ARM::SPRRegClass))
> +    return true;
>
> Wouldn't this also catch normal S-S copies? Are they not allowed?

The copy coalescer sometimes leaves SPR copies around, and these
would require post-regalloc handling. To make the solution pre-regalloc
only we need to handle them in the pass.

I've also found a problem with the VDUPfdf/VUPfqf expansion which caused
the tests to not pass when adding -verify-machineinstrs. I'm attaching
a patch that should fix this. It has no test case since the test in the
other patch already relies on this change.

- Silviu

> -----Original Message-----
> From: Jakob Stoklund Olesen [mailto:stoklund at 2pi.dk]
> Sent: 08 March 2013 21:55
> To: Silviu Baranga
> Cc: Tim Northover; James Molloy; Commit Messages and Patches for LLVM
> Subject: Re: [PATCH] Improve performance of vector code on A15
>
>
> On Mar 8, 2013, at 10:32 AM, Silviu Baranga <silbar01 at arm.com> wrote:
>
> > I've applied most of Tim's comments so I'm attaching the new version
> of
> > the patch. More tests would be good, but I still have to figure out a
> > meaningful way of writing them.
> >
> > The widened load has been changed from a vldr to an adr + vld1 (all
> lanes)
> > sequence in order to avoid accessing invalid memory.
>
> +  if (TargetRegisterInfo::isVirtualRegister(Reg))
> +    return MRI->getRegClass(Reg)->hasSuperClassEq(TRC);
> +  else
> +    return ARM::SPRRegClass.contains(Reg);
>
> Pasto.
>
> +  MI->getParent()->insertAfter(MI, NewMI);
> +  MI->getParent()->insertAfter(MI, AdrMI);
>
> You should simply use the variant of BuildMI that also inserts the new
> instructions.
>
> +        // Find the thing we're subreg copying out of - is it of the
> same
> +        // regclass as DPRMI? (i.e. a DPR or QPR).
> +        unsigned FullReg = SPRMI->getOperand(1).getReg();
> +        if (MRI->getRegClass(MI->getOperand(1).getReg()) ==
> +            MRI->getRegClass(FullReg)) {
>
> It's usually not correct to compare register classes by identity. You
> had a hasSuperClassEq() call above, I think you could use something
> similar here.
>
> +      if (MRI->getUniqueVRegDef(MI->getOperand(I).getReg())-
> >isImplicitDef())
>
> Don't assume getUniqueVRegDef() is non-null.
>
> Also, use getVRegDef while in SSA form.
>
> +  // The only way we can do a partial register update is through a
> COPY,
> +  // INSERT_SUBREG or REG_SEQUENCE.
> +  if (MI->isCopy() && usesRegClass(MI->getOperand(1),
> &ARM::SPRRegClass))
> +    return true;
>
> Wouldn't this also catch normal S-S copies? Are they not allowed?
>
> +void A15SDOptimizer::elideCopiesAndPHIs(MachineInstr *MI,
> +                                        SmallVector<MachineInstr*, 8>
> &Outs) {
>
> Always use SmallVectorImpl for references.
>
> +unsigned
> +A15SDOptimizer::createRegSequence(unsigned Reg1, unsigned Reg2,
> +                               MachineBasicBlock::iterator
> &InsertAfter) {
>
> Please use the normal pattern of passing an iterator to insert
> *before*, and don't pass it by reference.
>
> Many of these comments apply in multiple places. I only listed the
> first.
>
> /jakob
>
>


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vdupfdf-ops.diff
Type: application/octet-stream
Size: 900 bytes
Desc: vdupfdf-ops.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130311/77f1c1fb/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: a15-sd-preregalloc.diff
Type: application/octet-stream
Size: 26500 bytes
Desc: a15-sd-preregalloc.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130311/77f1c1fb/attachment-0001.obj>