[llvm-commits] [llvm] r112325 - in /llvm/trunk: include/llvm/LinkAllPasses.h include/llvm/Transforms/Scalar.h lib/Transforms/Scalar/ValuePropagation.cpp test/Transforms/ValuePropagation/ test/Transforms/ValuePropagation/dg.exp test/Transforms/ValuePropagation/phi.ll test/Transforms/ValuePropagation/select.ll

[Owen Anderson]

On Aug 30, 2010, at 3:46 PM, Chris Lattner wrote:
>> +++ llvm/trunk/lib/Transforms/Scalar/ValuePropagation.cpp Fri Aug 27 18:31:36 2010
>> @@ -0,0 +1,113 @@
>> +//===- ValuePropagation.cpp - Propagate information derived control flow --===//
>> +//
>> +//                     The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===----------------------------------------------------------------------===//
>> +//
>> +// This file implements the Value Propagation pass.
> 
> The name of this pass is very generic.  Can you think of something better?  It's not really propagating values afterall.

I don't really have a better idea for it.  I'm open to suggestions.

>> +  
>> +  if (changed)
>> +    for (Function::iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI)
>> +      SimplifyInstructionsInBlock(FI);
> 
> This loop is very odd... what does it do and why?  Why does it belong here?

The actual value propagation will tend to leave a lot of unused values and/or newly-constant PHI nodes sitting around.  These can't be removed/simplified during the main loop without invalidating the BasicBlock::iterator.

If ValuePropagation is run early enough, this probably isn't necessary as later runs of instcombine should clean up after it.  However, if we want to run it reasonably late, we probably want it to clean up after itself.

--Owen