<div dir="ltr"><div>Hi Chandler,<br><br></div>You're listed as an admin of Phab...and yet your email address(es) are shown as not verified, so I don't think it sent you mail about this patch!<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 28, 2017 at 5:55 PM, Sanjay Patel via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">spatel created this revision.<br>
Herald added subscribers: mcrosier, rengolin, aemerson.<br>
<br>
This is the fold that causes the infinite loop in BoringSSL (<a href="https://github.com/google/boringssl/blob/master/crypto/cipher/e_rc2.c" rel="noreferrer" target="_blank">https://github.com/google/<wbr>boringssl/blob/master/crypto/<wbr>cipher/e_rc2.c</a>) when we fix instcombine demanded bits to prefer 'not' ops as in <a href="https://reviews.llvm.org/D32255" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32255</a>.<br>
<br>
There are 2 or 3 problems with dyn_castNotVal, and I don't think we can reinstate <a href="https://reviews.llvm.org/D32255" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32255</a> until dyn_castNotVal is completely eliminated:<br>
<br>
1. As shown here, it transforms 'not' into random xor. This transform is harmful to SCEV and codegen because 'not' can often be folded while random xor cannot.<br>
2. It does not transform vector constants. This is actually a good thing, but if you don't believe the above argument, then we shouldn't have excluded vectors.<br>
3. It tries to avoid transforming not(not(X)). That's nice, but it doesn't match the greedy nature of instcombine. If we DeMorganize a pattern that has an extra 'not' in it:<br>
<br>
~(~(~X) & Y) --> (~X | ~Y)<br>
That's just another case of DeMorgan, so we should trust that we'll get that too:<br>
(~X | ~ Y) --> ~(X & Y)<br>
<br>
<br>
<a href="https://reviews.llvm.org/D32665" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32665</a><br>
<br>
Files:<br>
  lib/Transforms/InstCombine/<wbr>InstCombineAndOrXor.cpp<br>
  test/Transforms/InstCombine/<wbr>and-or.ll<br>
  test/Transforms/InstCombine/<wbr>assume2.ll<br>
<br>
</blockquote></div><br></div>