[PATCH] D39766: [InstCombine] Teach visitICmpInst to not break integer absolute value idioms

[Craig Topper via Phabricator via llvm-commits]

craig.topper updated this revision to Diff 122018.
craig.topper added a comment.

Add full context


https://reviews.llvm.org/D39766

Files:
  lib/Transforms/InstCombine/InstCombineCompares.cpp
  test/Transforms/InstCombine/icmp.ll


Index: test/Transforms/InstCombine/icmp.ll
===================================================================
--- test/Transforms/InstCombine/icmp.ll
+++ test/Transforms/InstCombine/icmp.ll
@@ -3270,3 +3270,19 @@
   %c = icmp sgt i8 %b2, %a2
   ret i1 %c
 }
+
+; Make sure InstCombine doesn't try too hard to simplify the icmp and break the abs idiom
+define i32 @abs_preserve(i32 %x) {
+; CHECK-LABEL: @abs_preserve(
+; CHECK-NEXT:    [[A:%.*]] = shl nsw i32 [[X:%.*]], 1
+; CHECK-NEXT:    [[C:%.*]] = icmp sgt i32 [[A]], -1
+; CHECK-NEXT:    [[NEGA:%.*]] = sub i32 0, [[A]]
+; CHECK-NEXT:    [[ABS:%.*]] = select i1 [[C]], i32 [[A]], i32 [[NEGA]]
+; CHECK-NEXT:    ret i32 [[ABS]]
+;
+  %a = mul nsw i32 %x, 2
+  %c = icmp sge i32 %a, 0
+  %nega = sub i32 0, %a
+  %abs = select i1 %c, i32 %a, i32 %nega
+  ret i32 %abs
+}
Index: lib/Transforms/InstCombine/InstCombineCompares.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -4462,11 +4462,28 @@
   // operands has at least one user besides the compare (the select),
   // which would often largely negate the benefit of folding anyway.
   if (I.hasOneUse())
-    if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin()))
-      if ((SI->getOperand(1) == Op0 && SI->getOperand(2) == Op1) ||
-          (SI->getOperand(2) == Op0 && SI->getOperand(1) == Op1))
+    if (SelectInst *SI = dyn_cast<SelectInst>(*I.user_begin())) {
+      Value *TrueVal = SI->getTrueValue();
+      Value *FalseVal = SI->getFalseValue();
+      if ((TrueVal == Op0 && FalseVal == Op1) ||
+          (FalseVal == Op0 && TrueVal == Op1))
         return nullptr;
 
+      // Prevent disturbing an absolute value operation as well.
+      const APInt *C;
+      if (match(Op1, m_APInt(C))) {
+        if ((Op0 == TrueVal && match(FalseVal, m_Neg(m_Specific(Op0)))) ||
+            (Op0 == FalseVal && match(TrueVal, m_Neg(m_Specific(Op0))))) {
+          ICmpInst::Predicate Pred = I.getPredicate();
+          if ((Pred == ICmpInst::ICMP_SGT &&
+               (C->isNullValue() || C->isAllOnesValue())) ||
+              (Pred == ICmpInst::ICMP_SLT &&
+               (C->isNullValue() || C->isOneValue())))
+            return nullptr;
+        }
+      }
+    }
+
   // Do this after checking for min/max to prevent infinite looping.
   if (Instruction *Res = foldICmpWithZero(I))
     return Res;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D39766.122018.patch
Type: text/x-patch
Size: 2468 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171108/6437e0c6/attachment.bin>