<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Dec 31, 2008, at 5:18 PM, Bill Wendling wrote:</div><blockquote type="cite"><div>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=61537&view=rev">http://llvm.org/viewvc/llvm-project?rev=61537&view=rev</a><br>Log:<br>Add transformation:<br><br> xor (or (icmp, icmp), true) -> and(icmp, icmp)<br><br>This is possible because of De Morgan's law.</div></blockquote><div><br></div>Nice. Note that this should only be done if the two compares each only ->hasOneUse(). Otherwise, you're introducing new compares.</div><div><br></div><div>Also, there is already code that handles a top-level not, please merge this into that code. The code is:</div><div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000"> </span>// Is this a ~ operation?</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">if</span> (Value *NotOp = dyn_castNotVal(&I)) {</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000"> </span>// ~(~X & Y) --> (X | ~Y) - De Morgan's Law</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(0, 116, 0); "><span style="color: #000000"> </span>// ~(~X | Y) === (X & ~Y) - De Morgan's Law</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; ">I'm not sure why that code exists and then the "bool not" code is separate, but it would be nice to merge them together. I realize this existed before your change :)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "><br></div></div><div><blockquote type="cite"><div>+ if (Op0I) {<br>+ Value *A, *B;<br>+ if (match(Op0I, m_Or(m_Value(A), m_Value(B)))) {<br>+ ICmpInst *AOp = dyn_cast<ICmpInst>(A);<br>+ ICmpInst *BOp = dyn_cast<ICmpInst>(B);</div></blockquote><div><br></div><div>Can you use m_ICmp here? Why not also handle fcmp? fcmp is invertible as well.</div><div><br></div><div>In practice, I think it would be better to simplify/merge this whole class of transformations by adding a new pair of static function:</div><div><br></div><div>bool isCheaplyInvertible(Value*);</div><div>Value *getChealyInvertedValue(Value*);</div><div><br></div><div>This would allow you to handle the icmp/fcmp/and "~" cases all identically instead of having slightly different copies of the various "demorgan" code.</div><div><br></div><div>-Chris</div></div></body></html>