PATCH: Early if-converter store if-conversion

[Arnold]

Sent from my iPhone

On Apr 25, 2013, at 3:42 AM, Andrew Trick <atrick at apple.com> wrote:

> 
> On Apr 23, 2013, at 1:22 PM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> 
>> This patch performs a similar optimization than my previous attempt did in SimplifyCFG.
>> 
>> This time we convert conditional stores in the early-if converter where we can use trace metrics to gauge the “closeness” of the two stores. If they are likely to be execute close together the cost of the second store should be cheap compared to mispredicting a branch.
>> 
>> This patch needs more tests and probably a target lowering call whether the (sub)target wants to if convert stores. But I wanted to get peoples opinions/input. Also it is not polished yet (so feel free to ignore formatting and similar concerns).
>> 
>> I have (correctness) tested the patch on test-suite+external and "-mllvm -x86-early-ifcvt=true -mllvm -stress-early-ifcvt=true” and observed no correctness regressions.
>> 
>> Performance will have to be analysed (give that the previous patch in simplify cfg did not cause too many regressions I don’t expect many to crop up here - but you never know). Jakob said that there are still some performance regressions with the early-if converter (without my patch). So, I wanted to wait with performance analysis until they are resolved.
>> 
>> I have however confirmed that we still get the 20% in hmmer ;)
>> 
>> 
>> Best,
>> Arnold
>> 
>> Attached are two patches: the first adds an api to targetinstrinfo to query and modify “register to memory stores”. During if-conversion we need to change the value register of such stores and need to compare two stores’ memory locations. The second implements the if-conversion in the early-if conversion pass.
>> 
>> <0001-TargetInstrInfo-Add-api-to-query-and-change-simple-r.patch><0002-Early-if-conversion-Implement-if-conversion-of-store.patch>
>> 
>> 
>> 
>> The thread following my simplify cfg commit http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130415/172206.html has a discussion why we rather wanted to do this in simplify cfg.
> 
> It's sad that you needed new hooks specifically for this transform.
> 

I believe that is the cost of doing transforms in codegen were you have target specific instructions and need to know their semantics. At least that has been my experience in the past.

Maybe it is not necessary in this case. I believe it is.

> I thought you could use mayStore along with an option to isSafeToMove to allow stores. Then MachineMemOperand to determine aliasing. Then maybe check if the aliasing stores have a single vreg that differs and cmove on that?


You are assuming that this vreg has the semantics of the value to be stored but it could mean something else. What about a theoretical "inc mem reg" increase a value in memory by register instruction.(without returning the value)
We have to make assumptions that a sane isa does not have such instructions then your approach probably works.

> 
> I could easily be missing something though...
> 
> -Andy