[llvm-commits] [llvm] r111665 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp test/Transforms/InstCombine/2010-08-19-StoreNarrowing.ll

[Chris Lattner]

On Aug 30, 2010, at 4:47 PM, Owen Anderson wrote:

> 
> On Aug 30, 2010, at 2:26 PM, Chris Lattner wrote:
>> 
>> 
>>> +  ConstantInt *CI1 =0, *CI2 = 0;
>>> +  Value *Ld = 0;
>>> +  if (getTargetData() &&
>>> +      match(SI.getValueOperand(),
>>> +            m_And(m_Or(m_Value(Ld), m_ConstantInt(CI1)), m_ConstantInt(CI2))) &&
>>> +      isa<LoadInst>(Ld) &&
>>> +      equivalentAddressValues(cast<LoadInst>(Ld)->getPointerOperand(), Ptr)) {
>> 
>> I don't see where you check to make sure you don't have something like:
>> 
>> x = load P
>> store 123 ->  Q    ;; might alias P!
>> x' = stuff(x)
>> store x' -> P
>> 
>> How do you know the loaded value hasn't been changed in memory?  Also, you don't check for a volatile load here.
> 
> Good catch.  Is this even reasonably fixable within the context of instcombine?  It looks like we'd want to use something like memdep to guarantee that there are no intervening writes to P.

This is best done in dag combine, where the chains tell you this.  Please revert your instcombine change.

-Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100830/d54f3355/attachment.html>