[llvm] r288855 - [InstSimplify] fixed (?) to not mutate icmps

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Tue Dec  6 16:09:52 2016
New Revision: 288855

URL: http://llvm.org/viewvc/llvm-project?rev=288855&view=rev
Log:
[InstSimplify] fixed (?) to not mutate icmps

As Eli noted in the post-commit thread for r288833, the use of
swapOperands() may not be allowed in InstSimplify, so I'm 
removing those calls here pending further review. 

The swap mutates the icmp, and there doesn't appear to be precedent
for instruction mutation in InstSimplify.

I didn't actually have any tests for those cases, so I'm adding
a few here. 


Modified:
    llvm/trunk/lib/Analysis/InstructionSimplify.cpp
    llvm/trunk/test/Transforms/InstSimplify/and-icmps-same-ops.ll
    llvm/trunk/test/Transforms/InstSimplify/or-icmps-same-ops.ll

Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=288855&r1=288854&r2=288855&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
+++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Tue Dec  6 16:09:52 2016
@@ -1523,11 +1523,8 @@ static Value *simplifyUnsignedRangeCheck
 static Value *simplifyAndOfICmpsWithSameOperands(ICmpInst *Op0, ICmpInst *Op1) {
   ICmpInst::Predicate Pred0, Pred1;
   Value *A ,*B;
-  match(Op0, m_ICmp(Pred0, m_Value(A), m_Value(B)));
-  if (match(Op1, m_ICmp(Pred1, m_Specific(B), m_Specific(A))))
-    Op1->swapOperands();
-
-  if (!match(Op1, m_ICmp(Pred1, m_Specific(A), m_Specific(B))))
+  if (!match(Op0, m_ICmp(Pred0, m_Value(A), m_Value(B))) ||
+      !match(Op1, m_ICmp(Pred1, m_Specific(A), m_Specific(B))))
     return nullptr;
 
   // We have (icmp Pred0, A, B) & (icmp Pred1, A, B).
@@ -1738,11 +1735,8 @@ Value *llvm::SimplifyAndInst(Value *Op0,
 static Value *simplifyOrOfICmpsWithSameOperands(ICmpInst *Op0, ICmpInst *Op1) {
   ICmpInst::Predicate Pred0, Pred1;
   Value *A ,*B;
-  match(Op0, m_ICmp(Pred0, m_Value(A), m_Value(B)));
-  if (match(Op1, m_ICmp(Pred1, m_Specific(B), m_Specific(A))))
-    Op1->swapOperands();
-
-  if (!match(Op1, m_ICmp(Pred1, m_Specific(A), m_Specific(B))))
+  if (!match(Op0, m_ICmp(Pred0, m_Value(A), m_Value(B))) ||
+      !match(Op1, m_ICmp(Pred1, m_Specific(A), m_Specific(B))))
     return nullptr;
 
   // We have (icmp Pred0, A, B) | (icmp Pred1, A, B).

Modified: llvm/trunk/test/Transforms/InstSimplify/and-icmps-same-ops.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/and-icmps-same-ops.ll?rev=288855&r1=288854&r2=288855&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/and-icmps-same-ops.ll (original)
+++ llvm/trunk/test/Transforms/InstSimplify/and-icmps-same-ops.ll Tue Dec  6 16:09:52 2016
@@ -1211,3 +1211,29 @@ define <2 x i1> @ult_ule_vec(<2 x i8> %a
   ret <2 x i1> %and
 }
 
+define i1 @ult_uge_swap(i8 %a, i8 %b) {
+; CHECK-LABEL: @ult_uge_swap(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ult i8 %a, %b
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp uge i8 %b, %a
+; CHECK-NEXT:    [[AND:%.*]] = and i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    ret i1 [[AND]]
+;
+  %cmp1 = icmp ult i8 %a, %b
+  %cmp2 = icmp uge i8 %b, %a
+  %and = and i1 %cmp1, %cmp2
+  ret i1 %and
+}
+
+define i1 @ult_ult_swap(i8 %a, i8 %b) {
+; CHECK-LABEL: @ult_ult_swap(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ult i8 %a, %b
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ult i8 %b, %a
+; CHECK-NEXT:    [[AND:%.*]] = and i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    ret i1 [[AND]]
+;
+  %cmp1 = icmp ult i8 %a, %b
+  %cmp2 = icmp ult i8 %b, %a
+  %and = and i1 %cmp1, %cmp2
+  ret i1 %and
+}
+

Modified: llvm/trunk/test/Transforms/InstSimplify/or-icmps-same-ops.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/or-icmps-same-ops.ll?rev=288855&r1=288854&r2=288855&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/or-icmps-same-ops.ll (original)
+++ llvm/trunk/test/Transforms/InstSimplify/or-icmps-same-ops.ll Tue Dec  6 16:09:52 2016
@@ -1211,3 +1211,29 @@ define <2 x i1> @ult_ule_vec(<2 x i8> %a
   ret <2 x i1> %or
 }
 
+define i1 @ult_ne_swap(i8 %a, i8 %b) {
+; CHECK-LABEL: @ult_ne_swap(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ult i8 %a, %b
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i8 %b, %a
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    ret i1 [[OR]]
+;
+  %cmp1 = icmp ult i8 %a, %b
+  %cmp2 = icmp ne i8 %b, %a
+  %or = or i1 %cmp1, %cmp2
+  ret i1 %or
+}
+
+define i1 @ult_ule_swap(i8 %a, i8 %b) {
+; CHECK-LABEL: @ult_ule_swap(
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ult i8 %a, %b
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp uge i8 %b, %a
+; CHECK-NEXT:    [[OR:%.*]] = or i1 [[CMP1]], [[CMP2]]
+; CHECK-NEXT:    ret i1 [[OR]]
+;
+  %cmp1 = icmp ult i8 %a, %b
+  %cmp2 = icmp uge i8 %b, %a
+  %or = or i1 %cmp1, %cmp2
+  ret i1 %or
+}
+