[llvm] r260695 - [InstCombine] Don't aggressively replace xor with icmp

[David Majnemer via llvm-commits]

Author: majnemer
Date: Fri Feb 12 12:12:38 2016
New Revision: 260695

URL: http://llvm.org/viewvc/llvm-project?rev=260695&view=rev
Log:
[InstCombine] Don't aggressively replace xor with icmp

For some cases, InstCombine replaces the sequence of xor/sub instruction
followed by cmp instruction into a single cmp instruction.

However, this replacement may result suboptimal result especially when
the xor/sub has more than one use, as discussed in
bug 26465 (https://llvm.org/bugs/show_bug.cgi?id=26465).

This patch make the replacement happen only when xor/sub has only one
use.

Differential Revision: http://reviews.llvm.org/D16915

Patch by Taewook Oh!

Added:
    llvm/trunk/test/Transforms/InstCombine/prevent-cmp-merge.ll
Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp?rev=260695&r1=260694&r2=260695&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp Fri Feb 12 12:12:38 2016
@@ -2212,27 +2212,30 @@ Instruction *InstCombiner::visitICmpInst
         }
         break;
       case Instruction::Xor:
-        // For the xor case, we can xor two constants together, eliminating
-        // the explicit xor.
-        if (Constant *BOC = dyn_cast<Constant>(BO->getOperand(1))) {
-          return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
-                              ConstantExpr::getXor(RHS, BOC));
-        } else if (RHSV == 0) {
-          // Replace ((xor A, B) != 0) with (A != B)
-          return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
-                              BO->getOperand(1));
+        if (BO->hasOneUse()) {
+          if (Constant *BOC = dyn_cast<Constant>(BO->getOperand(1))) {
+            // For the xor case, we can xor two constants together, eliminating
+            // the explicit xor.
+            return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+                ConstantExpr::getXor(RHS, BOC));
+          } else if (RHSV == 0) {
+            // Replace ((xor A, B) != 0) with (A != B)
+            return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+                BO->getOperand(1));
+          }
         }
         break;
       case Instruction::Sub:
-        // Replace ((sub A, B) != C) with (B != A-C) if A & C are constants.
-        if (ConstantInt *BOp0C = dyn_cast<ConstantInt>(BO->getOperand(0))) {
-          if (BO->hasOneUse())
+        if (BO->hasOneUse()) {
+          if (ConstantInt *BOp0C = dyn_cast<ConstantInt>(BO->getOperand(0))) {
+            // Replace ((sub A, B) != C) with (B != A-C) if A & C are constants.
             return new ICmpInst(ICI.getPredicate(), BO->getOperand(1),
-                                ConstantExpr::getSub(BOp0C, RHS));
-        } else if (RHSV == 0) {
-          // Replace ((sub A, B) != 0) with (A != B)
-          return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
-                              BO->getOperand(1));
+                ConstantExpr::getSub(BOp0C, RHS));
+          } else if (RHSV == 0) {
+            // Replace ((sub A, B) != 0) with (A != B)
+            return new ICmpInst(ICI.getPredicate(), BO->getOperand(0),
+                BO->getOperand(1));
+          }
         }
         break;
       case Instruction::Or:

Added: llvm/trunk/test/Transforms/InstCombine/prevent-cmp-merge.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/prevent-cmp-merge.ll?rev=260695&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/prevent-cmp-merge.ll (added)
+++ llvm/trunk/test/Transforms/InstCombine/prevent-cmp-merge.ll Fri Feb 12 12:12:38 2016
@@ -0,0 +1,41 @@
+; RUN: opt < %s -instcombine -S | FileCheck %s
+;
+; This test makes sure that InstCombine does not replace the sequence of
+; xor/sub instruction followed by cmp instruction into a single cmp instruction
+; if there is more than one use of xor/sub.
+
+define zeroext i1 @test1(i32 %lhs, i32 %rhs) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT: %xor = xor i32 %lhs, 5
+; CHECK-NEXT: %cmp1 = icmp eq i32 %xor, 10
+
+  %xor = xor i32 %lhs, 5
+  %cmp1 = icmp eq i32 %xor, 10
+  %cmp2 = icmp eq i32 %xor, %rhs
+  %sel = or i1 %cmp1, %cmp2
+  ret i1 %sel
+}
+
+define zeroext i1 @test2(i32 %lhs, i32 %rhs) {
+; CHECK-LABEL: @test2(
+; CHECK-NEXT: %xor = xor i32 %lhs, %rhs
+; CHECK-NEXT: %cmp1 = icmp eq i32 %xor, 0
+
+  %xor = xor i32 %lhs, %rhs
+  %cmp1 = icmp eq i32 %xor, 0
+  %cmp2 = icmp eq i32 %xor, 32
+  %sel = xor i1 %cmp1, %cmp2
+  ret i1 %sel
+}
+
+define zeroext i1 @test3(i32 %lhs, i32 %rhs) {
+; CHECK-LABEL: @test3(
+; CHECK-NEXT: %sub = sub nsw i32 %lhs, %rhs
+; CHECK-NEXT: %cmp1 = icmp eq i32 %sub, 0
+
+  %sub = sub nsw i32 %lhs, %rhs
+  %cmp1 = icmp eq i32 %sub, 0
+  %cmp2 = icmp eq i32 %sub, 31
+  %sel = or i1 %cmp1, %cmp2
+  ret i1 %sel
+}