<div dir="ltr">On Thu, Jul 11, 2013 at 12:49 AM, Duncan Sands <span dir="ltr"><<a href="mailto:duncan.sands@gmail.com" target="_blank">duncan.sands@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi David,<div><div class="h5"><br>
<br>
On 11/07/13 00:26, David Majnemer wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
InstCombine was pattern matching for (icmp (shl 1, X), Cst) to try to transform<br>
these icmps to true/false if we could prove that Cst must be in/out of range.<br>
<br>
However, there are two problems:<br>
1. This sort of thing belongs in InstSimplify<br>
2. This can be generalized further<br>
<br>
Attached is a patch that moves over these transforms to InstructionSimplify and<br>
handles arbitrary constants on the left hand side of the shift and constrains<br>
the possible range further by analyzing the nsw/nuw flags on the shift.<br>
</blockquote>
<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
--- lib/Analysis/<u></u>InstructionSimplify.cpp        (revision 186043)<br>
+++ lib/Analysis/<u></u>InstructionSimplify.cpp        (working copy)<br>
@@ -1995,6 +1995,27 @@<br>
         Lower = IntMin.sdiv(Val);<br>
         Upper = IntMax.sdiv(Val) + 1;<br>
       }<br>
+    } else if (match(LHS, m_Shl(m_ConstantInt(CI2), m_Value()))) {<br>
+      const APInt &CI2Value = CI2->getValue();<br>
+      if (cast<<u></u>OverflowingBinaryOperator>(<u></u>LHS)->hasNoUnsignedWrap()) {<br>
+        // 'shl nuw CI2, x' produces [CI2, CI2 << CLZ(CI2)]<br>
+        Lower = CI2Value;<br>
+        Upper = (CI2Value << CI2Value.countLeadingZeros()) + 1;<br>
+      } else if (cast<<u></u>OverflowingBinaryOperator>(<u></u>LHS)->hasNoSignedWrap()) {<br>
+        if (CI2->isNegative()) {<br>
+          // 'shl nsw CI2, x' [CI2 << (CLO(CI2)-1), CI2]<br>
+          Lower = (CI2Value << (CI2Value.countLeadingOnes() - 1));<br>
+          Upper = CI2Value + 1;<br>
+        } else {<br>
+          // 'shl nsw CI2, x' [CI2, CI2 << (CLZ(CI2)-1)]<br>
+          Lower = CI2Value;<br>
+          Upper = (CI2Value << (CI2Value.countLeadingZeros() - 1)) + 1;<br>
+        }<br>
</blockquote>
<br>
the cases above look OK, but<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+      } else {<br>
+        // 'shl CI2, x' produces [CI2, CI2 << (Width-1)]<br>
+        Lower = CI2Value;<br>
+        Upper = CI2Value.shl(Width - 1) + 1;<br>
</blockquote>
<br>
this one seems wrong to me.  Consider for example CI2 (in binary) equal to<br>
  10001000<br>
Then Lower=10001000, Upper=00000001, but a left-shift by 1 of CI2 is<br>
00010000 which is not in the range Lower .. Upper.<br></blockquote><div><br></div><div>Thanks, I've updated the patch so that we will set Lower to zero and Upper to CI2 << CLZ(CI2) which is conservative but, AFAIK, correct.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Ciao, Duncan.<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>