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

[Dan Gohman]

On Oct 24, 2009, at 6:20 PM, Chris Lattner wrote:

>
> On Oct 24, 2009, at 4:19 PM, 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.

>
>> +
>> +  // Now fix up users of the instructions in OrigHeader,  
>> insertting PHI nodes
>
> typo insertting.

Fixed.

>
>> +  // as necessary.
>> +  SSAUpdater SSA;
>> +  for (I = OrigHeader->begin(); I != E; ++I) {
>> +    Value *OrigHeaderVal = I;
>
> Most instructions only have uses within their block and these don't  
> need to be rewritten.  It would improve efficiency to do something  
> like this if it is safe:
>  if (I->hasOneUse() && I->use_back()->getParent() == I &&
>      !isa<PHINode>(I->use_back()))
>    continue;

The code already avoids calling RewriteUse in these cases.  Are you  
concerned
about the SSAUpdater::Initialize and AddAvailableValue calls?

Dan