PATCH: Early if-converter store if-conversion

[Hal Finkel]

----- Original Message -----
> From: "Arnold" <aschwaighofer at apple.com>
> To: "Andrew Trick" <atrick at apple.com>
> Cc: llvm-commits at cs.uiuc.edu
> Sent: Thursday, April 25, 2013 8:16:42 AM
> Subject: Re: PATCH: Early if-converter store if-conversion
> 
> 
> 
> 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.

Hrmm... don't do that, there are such instructions. As another example, PPC has byte-swapping stores.

 -Hal

> 
> > 
> > I could easily be missing something though...
> > 
> > -Andy
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>