[llvm-commits] [PATCH] Improve vmov domain conversion (followup)

[James Molloy]

Hi Jakob,

First patch: I originally added Ops for similarity between
analyzeVirtReg and analyzePhysReg. I've removed it from analyzePhysReg
as you asked. r163694.

Second patch: You're right; changed to computeRegisterLiveness and
committed in r163695.

Third; again you're right. I forgot to update this part after changing
the semantics of (what was) isRegisterLive. Previously this could never
return an unknown status.

I've attached an updated patch for this. Hopefully it should be clear of
such elementary errors :/

Thanks very much for reviewing all this so promptly, by the way!

Cheers,

James

On Tue, 2012-09-11 at 17:51 +0100, Jakob Stoklund Olesen wrote:
> On Sep 11, 2012, at 5:25 AM, James Molloy <james.molloy at arm.com> wrote:
> 
> > Hi Jakob,
> > 
> > Sorry this has takena  few days to get back to you.
> > 
> > I've moved analyzePhysReg into MachineOperandIteratorBase, and renamed
> > "RegInfo" to "VirtRegInfo" so the physical and virtual query functions
> > are complementary.
> > 
> > I hope this is more satisfactory? :)
> 
> Yes, please break it into three separate patches.
> 
> I don't think the 'Ops' argument to analyzePhysReg() is needed. Please remove that and go ahead and commit that part.
> 
> The MachineBasicBlock:isRegisterLive() function looks fine, except for the name. It doesn't return a bool, so it shouldn't be called is*. I would suggest computePhysRegLiveness() instead. Then go ahead and commit that part as well.
> 
> As for the ARM-specific part, I don't think that is safe. If you can't determine the liveness of the other lane, the transform should be aborted.
> 
> /jakob
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: arm-domain-conv-implicit-use.diff
Type: text/x-patch
Size: 5683 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120912/066a7f0a/attachment.bin>