PATCH: Early if-converter store if-conversion

Andrew Trick atrick at apple.com
Thu Apr 25 10:39:33 PDT 2013


On Apr 25, 2013, at 10:20 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:

> 
> On Apr 25, 2013, at 11:45 AM, Andrew Trick <atrick at apple.com> wrote:
>> You're right. We can't assume a store is repeatable. I thought we could weed out weird cases by checking MachineMemOperand properties, but on second thought probably not.
>> 
>> We should have an MI abstraction that provides simple load/store semantics. It would be nice if that abstraction were minimal and reused across phases that care: if-convert, scheduler, load/store optimizer. It strikes me as odd that we need MachineMemOperands, getLdStBaseRegImmOfs, and haveRegToMemStoresSameLocation.
> 
> 
> Are you thinking of a generic mi instruction like a PHI node that is lowered later. Or a more generic TTI api. I agree with you that we should avoid a proliferation of to specify apis.

I don't want any pseudo instructions, just a good TII API or maybe a transient proxy.

We should be preserving MachineMemOperands if possible. I think the idea is that if we drop them, then we don't get optimized.

There is a little confusion about whether MachineMemOperands should act as an abstraction that summarizes the load/store, or only hold information that cannot be recovered from the encoding. The current plan is to avoid any redundancy between MachineMemOperands and the underlying instruction. So I think we should have another abstraction for load/store semantics that asks the target for things like width, address/offset operands, and value operand, and gets things like volatile, IR value, and metadata from the MachineMemOperands.

I like your suggestions for simplifying the TII as a good start.

-Andy

> Back to the patch in question:
> 
> The reason I did not rely on MachineMemOperands for comparing memory locations is that I was not sure whether they would be there reliably (they can be omitted safely) in the cases we care about and we would loose opportunity because of there non-presence. Maybe, this is a non-concern for the cases we care about …
> 
> I think, I can get rid of haveRegToMemStoresSameLocation api call and just use the MachineMemOperand if we say we consider it a performance bug if the MachineMemOperand got lost along the way hampering our desire to if-convert a store in question.
> 
> We can also get rid of changeRegToMemStore ("change the register of a register to memory instruction"). After making sure that the MachineMemOperand is designating the same location, we only need to make sure that there is only one vreg different in the instruction (we already checked that it is a register to memory move using “isRegToMemStore”).
> 
> So we would be left with just “isRegToMemoryStore”.
> 
>> This shouldn't hold up the patch! I just want to eventually evolve toward a simpler API.
> 
> I also think as we move more optimizations to the machine level we want to make sure to keep apis as generic a possible. I have seen what happens otherwise … and it is not nice. Thanks for pushing me on this.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130425/988c93cc/attachment.html>


More information about the llvm-commits mailing list