[llvm-commits] [llvm] r85016 - /llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp

[Dan Gohman]

On Dec 20, 2009, at 11:38 PM, Chris Lattner <clattner at apple.com> wrote:

>
> On Oct 26, 2009, at 8:58 AM, Dan Gohman wrote:
>>>> Author: djg
>>>> Date: Sat Oct 24 18:19:52 2009
>>>> New Revision: 85016
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=85016&view=rev
>>>> Log:
>>>> Rewrite LoopRotation's SSA updating code using SSAUpdater.
>>>
>>> Awesome, I didn't remember loop rotate had its own ssa update code.
>>>
>>> If, in your travels, you run into any cases where SSAUpdate is a  
>>> significant compile time issue, please give me a testcase.  I have  
>>> many less-than-crazy ideas for speeding it up.
>>
>> One problem I'm seeing is redundant PHIs. Sphereflake for example
>> now gets code like this:
>>
>> %p.013.i = phi %struct.node_t* [ %p.0.be.i, %bb5.backedge.i ],  
>> [ %78, %bb9.i ] ; <%struct.node_t*> [#uses=1]
>> %p.012.i = phi %struct.node_t* [ %p.0.be.i, %bb5.backedge.i ],  
>> [ %78, %bb9.i ] ; <%struct.node_t*> [#uses=1]
>> %p.011.i = phi %struct.node_t* [ %p.0.be.i, %bb5.backedge.i ],  
>> [ %78, %bb9.i ] ; <%struct.node_t*> [#uses=1]
>> %p.010.i = phi %struct.node_t* [ %p.0.be.i, %bb5.backedge.i ],  
>> [ %78, %bb9.i ] ; <%struct.node_t*> [#uses=1]
>> %p.09.i = phi %struct.node_t* [ %p.0.be.i, %bb5.backedge.i ],  
>> [ %78, %bb9.i ] ; <%struct.node_t*> [#uses=7]
>>
>> Indvars can't eliminate these because they aren't a function of the
>> canonical induction variable.   These PHIs stick around and end up
>> being allocated registers, which is suboptimal.
>
> I finally found a good example that demonstrated this, PR5837, which  
> is now fixed.  I recall that you added phi merging code to  
> simplifycfg or instcombine somewhere, do you think it makes sense to  
> remove this (potentially really expensive) optimization now?

It's not really expensive; it's proportional to the size of the input  
in the worst case (ignoring worst case hash collisions).

There's no other optimization that removes redundant phis like this.   
I guess if front-ends never emit phis and if all optimization passes  
which emit phis check existing phis (the ones I'm familiar with all do  
at this point), then this isn't needed. Otherwise it's still useful.

Dan