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

[Jakob Stoklund Olesen]

On Sep 4, 2012, at 1:25 AM, James Molloy <james.molloy at arm.com> wrote:

> It defines a generic "isRegisterLive()" function that takes a
> MachineInstr and register and will check if the register is live at
> arrival at the MachineInstr.
> 
> It is O(N). :( . I couldn't see a better way of doing it though.
> 
> Could you please review?

Hi James,

Thanks for working on this!

The general approach is fine, but as I told Tim, you should limit the scan to a neighbourhood of MI to avoid the O(N) behaviour on large basic blocks. I would suggest looking at the 10 instructions before and the 10 instructions after MI. We can negotiate a higher limit, if you need it ;-)

Also note that when dealing with overlapping registers, liveness isn't a binary property. Your function could return one of:

- Reg is live.
- Reg is not live, but some overlapping register is live.
- Reg is not live, and no overlapping registers are live. (Thus, it is safe to clobber Reg).
- Couldn't tell from local neighbourhood.

You could formulate this as answering two questions:

1. Is Reg definitely live?
2. Is it definitely safe to clobber Reg?

Usually, code is looking for the answer to one of those questions. In the vmov case, it seems you need both.


Regarding your patch, please don't use reverse_iterator. It is a quite confusing class that should only be used when necessary. (Stepping a reverse_iterator backwards is a double negative).

We've seen other code recently that calls MI->definesRegister() and MI->killsRegister() on the same instruction. I think it would make sense to add a MachineOperandIteratorBase::analyzePhysReg(Reg) function that scans the instruction operands once, and returns these flags:

Clobbers - Reg or an overlapping register is defined, or a regmask clobbers Reg. (Same as MI->clobbersPhysReg()).
Defines - Reg or a super-register is defined. (Same as MI->definesRegister()).
Reads - An operand reads Reg or a super-register. (As determined by readsReg()).
ReadsOverlap - An operand reads Reg or an overlapping register.
DefinesDead - All defs of Reg or a super-register are dead.
Kills - There is a kill of Reg or a super-register. (Same as MI->killsRegister()).

/jakob