<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 30, 2010, at 4:47 PM, Owen Anderson wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 30, 2010, at 2:26 PM, Chris Lattner wrote:</div><blockquote type="cite"><div><font class="Apple-style-span"><br></font><br><blockquote type="cite">+ ConstantInt *CI1 =0, *CI2 = 0;<br></blockquote><blockquote type="cite">+ Value *Ld = 0;<br></blockquote><blockquote type="cite">+ if (getTargetData() &&<br></blockquote><blockquote type="cite">+ match(SI.getValueOperand(),<br></blockquote><blockquote type="cite">+ m_And(m_Or(m_Value(Ld), m_ConstantInt(CI1)), m_ConstantInt(CI2))) &&<br></blockquote><blockquote type="cite">+ isa<LoadInst>(Ld) &&<br></blockquote><blockquote type="cite">+ equivalentAddressValues(cast<LoadInst>(Ld)->getPointerOperand(), Ptr)) {<br></blockquote><br>I don't see where you check to make sure you don't have something like:<br><br>x = load P<br>store 123 -> Q ;; might alias P!<br>x' = stuff(x)<br>store x' -> P<br><br>How do you know the loaded value hasn't been changed in memory? Also, you don't check for a volatile load here.<br></div></blockquote></div><br><div>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.</div></div></blockquote><br></div><div>This is best done in dag combine, where the chains tell you this. Please revert your instcombine change.</div><div><br></div><div>-Chris</div><br></body></html>