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

Dan Gohman gohman at apple.com
Wed Sep 1 10:16:16 PDT 2010


On Aug 30, 2010, at 5:17 PM, Chris Lattner wrote:

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

Hi Chris, is the memory dependence issue the only reason to prefer to use
dagcombine here instead of instcombine?  This can be solved in instcombine
also using FindAvailableLoadedValue.

Narrowing stores and eliminating loads can open up a variety of optimization
opportunities. This suggests that instcombine is a better place for this
optimization.

Dan





More information about the llvm-commits mailing list