[llvm] cc9c545 - [InstCombine] generalize subtract with 'not' operands; 2nd try

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2021-08-23T17:06:51-04:00
New Revision: cc9c545fb42107fdba94e242178c4acc964ca18b

URL: https://github.com/llvm/llvm-project/commit/cc9c545fb42107fdba94e242178c4acc964ca18b
DIFF: https://github.com/llvm/llvm-project/commit/cc9c545fb42107fdba94e242178c4acc964ca18b.diff

LOG: [InstCombine] generalize subtract with 'not' operands; 2nd try

This is a re-try of 3aa009cc87e3 which was reverted at
9577fac0fddf because it caused an infinite loop.

For the extra test case, either re-ordering the transforms
or adding the extra clause to avoid sub-of-sub is enough
to prevent the infinite compile, but I'm doing both to be
safer.

Original commit message:
The motivation was to get min/max intrinsics to parity
with cmp+select idioms, but this unlocks a few more
folds because isFreeToInvert recognizes add/sub with
constants too.

In the min/max example, we have too many extra uses
for smaller folds to improve things, but this fold
is able to eliminate uses even though we can't reduce
the number of instructions.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
    llvm/test/Transforms/InstCombine/reassociate-nuw.ll
    llvm/test/Transforms/InstCombine/sub.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index aa4f0bdffe10..153d517ac770 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1828,12 +1828,8 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   if (match(Op0, m_AllOnes()))
     return BinaryOperator::CreateNot(Op1);
 
-  // (~X) - (~Y) --> Y - X
-  Value *X, *Y;
-  if (match(Op0, m_Not(m_Value(X))) && match(Op1, m_Not(m_Value(Y))))
-    return BinaryOperator::CreateSub(Y, X);
-
   // (X + -1) - Y --> ~Y + X
+  Value *X, *Y;
   if (match(Op0, m_OneUse(m_Add(m_Value(X), m_AllOnes()))))
     return BinaryOperator::CreateAdd(Builder.CreateNot(Op1), X);
 
@@ -1854,6 +1850,17 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
     return BinaryOperator::CreateSub(X, Add);
   }
 
+  // (~X) - (~Y) --> Y - X
+  // This is placed after the other reassociations and explicitly excludes a
+  // sub-of-sub pattern to avoid infinite looping.
+  if (isFreeToInvert(Op0, Op0->hasOneUse()) &&
+      isFreeToInvert(Op1, Op1->hasOneUse()) &&
+      !match(Op0, m_Sub(m_ImmConstant(), m_Value()))) {
+    Value *NotOp0 = Builder.CreateNot(Op0);
+    Value *NotOp1 = Builder.CreateNot(Op1);
+    return BinaryOperator::CreateSub(NotOp1, NotOp0);
+  }
+
   auto m_AddRdx = [](Value *&Vec) {
     return m_OneUse(m_Intrinsic<Intrinsic::vector_reduce_add>(m_Value(Vec)));
   };

diff  --git a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
index 2e4224b11ccf..ce7cc81b60e6 100644
--- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
@@ -1157,8 +1157,8 @@ define i8 @freeToInvertSub(i8 %x, i8 %y, i8 %z) {
 ; CHECK-NEXT:    call void @use(i8 [[NX]])
 ; CHECK-NEXT:    call void @use(i8 [[NY]])
 ; CHECK-NEXT:    call void @use(i8 [[NZ]])
-; CHECK-NEXT:    [[M:%.*]] = call i8 @llvm.umax.i8(i8 [[NX]], i8 [[NY]])
-; CHECK-NEXT:    [[SUB:%.*]] = sub i8 [[NZ]], [[M]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.umin.i8(i8 [[X]], i8 [[Y]])
+; CHECK-NEXT:    [[SUB:%.*]] = sub i8 [[TMP1]], [[Z]]
 ; CHECK-NEXT:    ret i8 [[SUB]]
 ;
   %nx = xor i8 %x, -1

diff  --git a/llvm/test/Transforms/InstCombine/reassociate-nuw.ll b/llvm/test/Transforms/InstCombine/reassociate-nuw.ll
index a40d427fd29f..6045f1ec8806 100644
--- a/llvm/test/Transforms/InstCombine/reassociate-nuw.ll
+++ b/llvm/test/Transforms/InstCombine/reassociate-nuw.ll
@@ -79,9 +79,8 @@ define i32 @reassoc_x2_mul_nuw(i32 %x, i32 %y) {
 
 define i32 @reassoc_x2_sub_nuw(i32 %x, i32 %y) {
 ; CHECK-LABEL: @reassoc_x2_sub_nuw(
-; CHECK-NEXT:    [[SUB0:%.*]] = add i32 [[X:%.*]], -4
-; CHECK-NEXT:    [[SUB1:%.*]] = add i32 [[Y:%.*]], -8
-; CHECK-NEXT:    [[SUB2:%.*]] = sub nuw i32 [[SUB0]], [[SUB1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i32 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[SUB2:%.*]] = add i32 [[TMP1]], 4
 ; CHECK-NEXT:    ret i32 [[SUB2]]
 ;
   %sub0 = sub nuw i32 %x, 4

diff  --git a/llvm/test/Transforms/InstCombine/sub.ll b/llvm/test/Transforms/InstCombine/sub.ll
index 878061b9296e..74319e403193 100644
--- a/llvm/test/Transforms/InstCombine/sub.ll
+++ b/llvm/test/Transforms/InstCombine/sub.ll
@@ -1109,14 +1109,9 @@ define i32 @test57(i32 %A, i32 %B) {
 @dummy_global2 = external global i8*
 
 define i64 @test58([100 x [100 x i8]]* %foo, i64 %i, i64 %j) {
-; Note the reassociate pass and another instcombine pass will further optimize this to
-; "%sub = i64 %i, %j, ret i64 %sub"
-; gep1 and gep2 have only one use
 ; CHECK-LABEL: @test58(
-; CHECK-NEXT:    [[GEP1_OFFS:%.*]] = add nsw i64 [[I:%.*]], 4200
-; CHECK-NEXT:    [[GEP2_OFFS:%.*]] = add nsw i64 [[J:%.*]], 4200
-; CHECK-NEXT:    [[GEPDIFF:%.*]] = sub nsw i64 [[GEP1_OFFS]], [[GEP2_OFFS]]
-; CHECK-NEXT:    ret i64 [[GEPDIFF]]
+; CHECK-NEXT:    [[TMP1:%.*]] = sub i64 [[I:%.*]], [[J:%.*]]
+; CHECK-NEXT:    ret i64 [[TMP1]]
 ;
   %gep1 = getelementptr inbounds [100 x [100 x i8]], [100 x [100 x i8]]* %foo, i64 0, i64 42, i64 %i
   %gep2 = getelementptr inbounds [100 x [100 x i8]], [100 x [100 x i8]]* %foo, i64 0, i64 42, i64 %j