[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

[Dan Gohman]

On Jan 3, 2011, at 3:24 PM, Chris Lattner wrote:

> 
> 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?

Ok, I see it now. So it would theoretically be possible to use
isIdenticalToWhenDefined instead, along with intersectOptionalDataWith,
to be slightly more aggressive, but it's probably not important.

Dan