[PATCH] D38637: [InstSimplify] don't let poison inhibit an easy fold

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
Herald added a subscriber: mcrosier.

https://reviews.llvm.org/D38591 offers one way to avoid the assert in PR34838, but we could just explicitly handle this pattern in InstSimplify to make life easier for InstCombine. This also avoids using computeKnownBits() if we don't have to and gets known bad code reduced faster, so we're not wasting time on it.


https://reviews.llvm.org/D38637

Files:
  lib/Analysis/InstructionSimplify.cpp
  test/Transforms/InstSimplify/icmp-constant.ll


Index: test/Transforms/InstSimplify/icmp-constant.ll
===================================================================
--- test/Transforms/InstSimplify/icmp-constant.ll
+++ test/Transforms/InstSimplify/icmp-constant.ll
@@ -571,3 +571,29 @@
   ret <2 x i1> %cmp
 }
 
+; PR34838 - https://bugs.llvm.org/show_bug.cgi?id=34838
+; The shift is known to create poison, but that doesn't mean we can't simplify the cmp.
+
+define i1 @ne_shl_low_bits_set(i8 %x) {
+; CHECK-LABEL: @ne_shl_low_bits_set(
+; CHECK-NEXT:    ret i1 true
+;
+  %zx = zext i8 %x to i16      ; zx  = 0x00xx
+  %xor = xor i16 %zx, 32767    ; xor = 0x7fyy
+  %sub = sub nsw i16 %zx, %xor ; sub = 0x80zz  (the top bit is known one)
+  %sh = shl nsw i16 %sub, 2    ; oops! this shl can't be nsw; that's POISON
+  %cmp = icmp ne i16 %sh, 1
+  ret i1 %cmp
+}
+
+define i1 @eq_shl_low_bits_set(i8 %x) {
+; CHECK-LABEL: @eq_shl_low_bits_set(
+; CHECK-NEXT:    ret i1 false
+;
+  %clear_high_bit = and i8 %x, 127
+  %set_next_high_bits = or i8 %clear_high_bit, 112 ; 0x70
+  %poison_shift = shl nsw i8 %set_next_high_bits, 3
+  %cmp = icmp eq i8 %poison_shift, 15
+  ret i1 %cmp
+}
+
Index: lib/Analysis/InstructionSimplify.cpp
===================================================================
--- lib/Analysis/InstructionSimplify.cpp
+++ lib/Analysis/InstructionSimplify.cpp
@@ -2405,12 +2405,13 @@
   if (!match(RHS, m_APInt(C)))
     return nullptr;
 
+  Type *CmpTy = GetCompareTy(RHS);
   // Rule out tautological comparisons (eg., ult 0 or uge 0).
   ConstantRange RHS_CR = ConstantRange::makeExactICmpRegion(Pred, *C);
   if (RHS_CR.isEmptySet())
-    return ConstantInt::getFalse(GetCompareTy(RHS));
+    return ConstantInt::getFalse(CmpTy);
   if (RHS_CR.isFullSet())
-    return ConstantInt::getTrue(GetCompareTy(RHS));
+    return ConstantInt::getTrue(CmpTy);
 
   // Find the range of possible values for binary operators.
   unsigned Width = C->getBitWidth();
@@ -2428,9 +2429,22 @@
 
   if (!LHS_CR.isFullSet()) {
     if (RHS_CR.contains(LHS_CR))
-      return ConstantInt::getTrue(GetCompareTy(RHS));
+      return ConstantInt::getTrue(CmpTy);
     if (RHS_CR.inverse().contains(LHS_CR))
-      return ConstantInt::getFalse(GetCompareTy(RHS));
+      return ConstantInt::getFalse(CmpTy);
+  }
+
+  // Shift-left doesn't easily conform to range reduction, but we can still
+  // check if the inserted zero bits make this comparison true or false.
+  const APInt *ShiftAmtC;
+  if (match(LHS, m_Shl(m_Value(), m_APInt(ShiftAmtC))) &&
+      C->lshr(*ShiftAmtC).shl(*ShiftAmtC) != *C) {
+    // icmp eq (shl X, ShiftAmtC), C --> false if any low bits of C are set
+    if (Pred == ICmpInst::ICMP_EQ)
+      return ConstantInt::getFalse(CmpTy);
+    // icmp ne (shl X, ShiftAmtC), C --> true if any low bits of C are set
+    if (Pred == ICmpInst::ICMP_NE)
+      return ConstantInt::getTrue(CmpTy);
   }
 
   return nullptr;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38637.118034.patch
Type: text/x-patch
Size: 2899 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171006/58219147/attachment.bin>