[llvm] e465f9c - [InstCombine] Negator: - (C - %x) --> %x - C (PR47997)

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-11-03T16:06:51+03:00
New Revision: e465f9c3036b28bdbdbef03eee8fd022a3b6dcbf

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

LOG: [InstCombine] Negator: - (C - %x) --> %x - C (PR47997)

This relaxes one-use restriction on that `sub` fold,
since apparently the addition of Negator broke
preexisting `C-(C2-X) --> X+(C-C2)` (with C=0) fold.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
    llvm/test/Transforms/InstCombine/icmp.ll
    llvm/test/Transforms/InstCombine/sub-of-negatible.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
index b321eab01d7a..5a72547ce519 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
@@ -219,19 +219,22 @@ LLVM_NODISCARD Value *Negator::visitImpl(Value *V, unsigned Depth) {
     break; // Other instructions require recursive reasoning.
   }
 
+  if (I->getOpcode() == Instruction::Sub &&
+      (I->hasOneUse() || (isa<Constant>(I->getOperand(0)) &&
+                          !isa<ConstantExpr>(I->getOperand(0))))) {
+    // `sub` is always negatible.
+    // However, only do this either if the old `sub` doesn't stick around, or
+    // it was subtracting from a constant. Otherwise, this isn't profitable.
+    return Builder.CreateSub(I->getOperand(1), I->getOperand(0),
+                             I->getName() + ".neg");
+  }
+
   // Some other cases, while still don't require recursion,
   // are restricted to the one-use case.
   if (!V->hasOneUse())
     return nullptr;
 
   switch (I->getOpcode()) {
-  case Instruction::Sub:
-    // `sub` is always negatible.
-    // But if the old `sub` sticks around, even thought we don't increase
-    // instruction count, this is a likely regression since we increased
-    // live-range of *both* of the operands, which might lead to more spilling.
-    return Builder.CreateSub(I->getOperand(1), I->getOperand(0),
-                             I->getName() + ".neg");
   case Instruction::SDiv:
     // `sdiv` is negatible if divisor is not undef/INT_MIN/1.
     // While this is normally not behind a use-check,

diff  --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll
index 27b85aae180f..124195a8469a 100644
--- a/llvm/test/Transforms/InstCombine/icmp.ll
+++ b/llvm/test/Transforms/InstCombine/icmp.ll
@@ -3831,9 +3831,7 @@ define i1 @pr47997(i32 %arg) {
 ; CHECK-NEXT:    store i32 [[I]], i32* @x, align 4
 ; CHECK-NEXT:    [[I1:%.*]] = sub nsw i32 1, [[ARG]]
 ; CHECK-NEXT:    store i32 [[I1]], i32* @y, align 4
-; CHECK-NEXT:    [[I2:%.*]] = sub nsw i32 0, [[I1]]
-; CHECK-NEXT:    [[I3:%.*]] = icmp eq i32 [[I]], [[I2]]
-; CHECK-NEXT:    ret i1 [[I3]]
+; CHECK-NEXT:    ret i1 true
 ;
 bb:
   %i = add nsw i32 %arg, -1

diff  --git a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
index 40f51d2c75a2..bd8e9d23d73f 100644
--- a/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
+++ b/llvm/test/Transforms/InstCombine/sub-of-negatible.ll
@@ -195,10 +195,10 @@ define i8 @neg_of_sub_from_constant(i8 %x) {
 
 define i8 @neg_of_sub_from_constant_multi_use(i8 %x) {
 ; CHECK-LABEL: @neg_of_sub_from_constant_multi_use(
-; CHECK-NEXT:    [[S:%.*]] = sub i8 42, [[X:%.*]]
+; CHECK-NEXT:    [[S_NEG:%.*]] = add i8 [[X:%.*]], -42
+; CHECK-NEXT:    [[S:%.*]] = sub i8 42, [[X]]
 ; CHECK-NEXT:    call void @use8(i8 [[S]])
-; CHECK-NEXT:    [[R:%.*]] = sub i8 0, [[S]]
-; CHECK-NEXT:    ret i8 [[R]]
+; CHECK-NEXT:    ret i8 [[S_NEG]]
 ;
   %s = sub i8 42, %x
   call void @use8(i8 %s)