[llvm-commits] [llvm] r122715 - in /llvm/trunk: lib/Transforms/Scalar/EarlyCSE.cpp test/Transforms/EarlyCSE/ test/Transforms/EarlyCSE/basic.ll test/Transforms/EarlyCSE/dg.exp

[Chris Lattner]

On Jan 3, 2011, at 3:15 PM, Dan Gohman wrote:

> 
> On Jan 2, 2011, at 3:04 PM, Chris Lattner wrote:
>> 
>> +    // See if the instruction has an available value.  If so, use it.
>> +    if (Instruction *V = AvailableValues->lookup(InstValue::get(Inst))) {
>> +      Inst->replaceAllUsesWith(V);
>> +      Inst->eraseFromParent();
>> +      Changed = true;
>> +      continue;
>> +    }
> 
> Hi Chris,
> 
> Since optional flags such as nsw, inbounds, etc. are not currently being hashed,
> this code could replace a value with another value with a proper superset of
> flags, which is not valid in general. One way to fix this would be to call
> intersectOptionalDataWith here.

This will only replace one instruction with another if they pass the "Instruction::isIdenticalTo" predicate.  I think that checks flags, so this should be ok (we'll just get a hash collision, which is fine).  Does this make sense?

-Chris