[llvm-commits] [PATCH 2/2] Convert VLDR/VSTR on demand for base update optimization

[David Peixotto]

Ok, I've addressed the comments from the review. I've attached a new patch
to this email.

I also fixed all the issues found by -verify-machineinstrs. Those fixes
belonged the in other patch in this patch set (see [PATCH 1/2] Improve
optimization to fold base update into LDM/STM), and the patch in this email
already depends on that other patch

I've also uploaded a new version of the other patch. Once it is reviewed and
approved both patches are ready to be applied.

Thanks,
Dave

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of David Peixotto
> Sent: Thursday, September 06, 2012 6:44 PM
> To: 'Jim Grosbach'
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH 2/2] Convert VLDR/VSTR on demand for
> base update optimization
> 
> Thanks for the feedback Jim!
> 
> Thanks for the tip about -verify-machineinstrs. It found a few things to
> complain about and I clean those up. I'll also apply your suggestions
below
> and submit a new patch. Thanks!
> 
> -Dave
> 
> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 
> -----Original Message-----
> From: Jim Grosbach [mailto:grosbach at apple.com]
> Sent: Wednesday, September 05, 2012 5:21 PM
> To: David Peixotto
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH 2/2] Convert VLDR/VSTR on demand for
> base update optimization
> 
> Nice!
> 
> Style note:
> +static MachineInstr* convertVLSRtoVLSMIA(MachineBasicBlock &MBB,
> +                                         MachineInstr *MI,
> +                                         const TargetInstrInfo *TII) {
> 
> The '{' should go on the preceding line, not on a new line. Similarly for
the
> other functions.
> 
> 
> In convertVLSRtoVLSMIA:
> 
> +  MachineBasicBlock::iterator MBBI(MI);  MachineInstrBuilder MIB =
> + BuildMI(MBB, MBBI, MI->getDebugLoc(),
> +                                    TII->get(NewOpc));
> +
> +  assert(MI->getOperand(2).isImm() && MI->getOperand(2).getImm() == 0
> &&
> +         "VLDR has non-zero pre-increment value");
> + MIB.addOperand(MI->getOperand(1)); // Base address
> + MIB.addOperand(MI->getOperand(3)); // Predicate Imm
> + MIB.addOperand(MI->getOperand(4)); // Predicate Register
> + MIB.addOperand(MI->getOperand(0)); // VPR register
> 
> The VPR register should probably be marked as a def for loads.
> 
> In convertVLSMIAtoVLSR:
> 
> +  MachineBasicBlock::iterator MBBI(MI);  MachineInstrBuilder MIB =
> + BuildMI(MBB, MBBI, MI->getDebugLoc(),
> +                                    TII->get(NewOpc));
> +
> +  // There should be only one VPR register since this instruction was
> + // originally converted from a VLDR/VSTR
> + MIB.addOperand(MI->getOperand(3)); // VPR register
> + MIB.addOperand(MI->getOperand(0)); // Base address
> +  MIB.addImm(0);                     // Offset
> +  MIB.addOperand(MI->getOperand(1)); // Predicate Imm
> + MIB.addOperand(MI->getOperand(2)); // Predicate Register
> +
> 
> The VPR register should definitely be marked as a def for loads.
> 
> If you haven't already, make sure to run tests with the machine verifier
> enabled after the pass runs. It'l help catch lots of things.
> 
> 
> LGTM with those updates.
> 
> -Jim
> 
> 
> On Aug 31, 2012, at 11:02 AM, David Peixotto <dpeixott at codeaurora.org>
> wrote:
> 
> > Please review the attached patch. Thanks!
> >
> > This commit allows the base-address update optimization to work for
> > VLDR and VSTR. There is no writeback version of VLDR/VSTR, so we
> > cannot directly fold the update of the base address into the
> > instruction itself. We must first transform the VLDR/VSTR to VLDM/VSTM
> > and then use the writeback version of those instructions. For example
> > we perform the following transformation,
> >
> > VLDR D0, [R0]
> > ADD  R0, R0, #8
> > ==>
> > VLDM R0!, {D0}
> >
> > The VLDR/VSTR instructions will only be changed to VLDM/VSTM if an
> > update of the base address register can be folded into the
> > instruction. Other VLDR/VSTR instructions are left undisturbed.
> >
> > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > hosted by The Linux Foundation
> >
> >
> >
> > <0002-Convert-VLDR-VSTR-on-demand-for-base-update-
> optimiza.patch>_____
> > __________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Convert-VLDR-VSTR-on-demand-for-base-update-optimiza.patch
Type: application/octet-stream
Size: 11683 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120908/0c80b661/attachment.obj>