[PATCH] D39536: [PowerPC] Eliminate redundant register copys after register allocation

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 7 06:20:06 PST 2017


nemanjai added a comment.

In https://reviews.llvm.org/D39536#917677, @inouehrs wrote:

> > If we're adding a new peephole, it likely won't only be used for this one purpose. A more general name might be in order.
>
> I changed the pass name. Do you have any suggestion on an alternative name?


If you agree that this would likely be used for other purposes, why not just name it `PPCPostRAPeephole`? Otherwise if you think this will likely only be used for this purpose, I suppose the name is fine.

> 
> 
>> This seems a bit fragile. I'm not sure there's any hard guarantee that the flag will be on the last source operand.
> 
> I modified to check the flag for both input operands.

Thanks.

> 
> 
>> These loops as well as a check for whether the instruction is a call seem a little odd. Why is it inadequate to use MachineInstr::readsRegister() and MachineInstr::modifiesRegister() for these purposes. Given the TRI, they should return the correct value for any instruction (including calls).
> 
> `readsRegister` checks whether the instruction reads the specified register or its super register, but it does not check about sub register access. I need to check sub register access for safety.

Hmm.. that certainly doesn't seem to be the case by just looking at the implementation. There's a check for `Found = TRI->regsOverlap(MOReg, Reg);` and I would assume this would return true for any overlap. But of course, this is based on just a cursory look at the code and I may be missing something.


https://reviews.llvm.org/D39536





More information about the llvm-commits mailing list