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

[David Peixotto]

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