[llvm] 84cdccc - [InstCombine] try to eliminate an instruction in min/max -> abs fold

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2021-04-09T10:34:03-04:00
New Revision: 84cdccc9dc68ce11b8eb9a0cbb4333b7f3cb1c1e

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

LOG: [InstCombine] try to eliminate an instruction in min/max -> abs fold

As suggested in the review thread for 5094e12 and seen in the
motivating example from https://llvm.org/PR49885, it's not
clear if we have a way to create the optimal code without
this heuristic.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
    llvm/test/Transforms/InstCombine/minmax-intrinsics.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index b139960b478e6..78f45116063bc 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -916,12 +916,21 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     // umax(X, -X) --> -abs(X)
     // umin(X, -X) --> abs(X)
     if (isKnownNegation(I0, I1)) {
+      // We can choose either operand as the input to abs(), but if we can
+      // eliminate the only use of a value, that's better for subsequent
+      // transforms/analysis.
+      if (I0->hasOneUse() && !I1->hasOneUse())
+        std::swap(I0, I1);
+
       // This is some variant of abs(). See if we can propagate 'nsw' to the abs
       // operation and potentially its negation.
       bool IntMinIsPoison = isKnownNegation(I0, I1, /* NeedNSW */ true);
       Value *Abs = Builder.CreateBinaryIntrinsic(
           Intrinsic::abs, I0,
           ConstantInt::getBool(II->getContext(), IntMinIsPoison));
+
+      // We don't have a "nabs" intrinsic, so negate if needed based on the
+      // max/min operation.
       if (IID == Intrinsic::smin || IID == Intrinsic::umax)
         Abs = Builder.CreateNeg(Abs, "nabs", /* NUW */ false, IntMinIsPoison);
       return replaceInstUsesWith(CI, Abs);

diff  --git a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
index 62800424df70e..a1185c50bc8bc 100644
--- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
@@ -692,10 +692,9 @@ define i8 @umin_negation(i8 %x) {
 
 define i8 @smax_negation_uses(i8 %x, i8 %y) {
 ; CHECK-LABEL: @smax_negation_uses(
-; CHECK-NEXT:    [[S1:%.*]] = sub i8 [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT:    [[S2:%.*]] = sub i8 [[Y]], [[X]]
+; CHECK-NEXT:    [[S2:%.*]] = sub i8 [[Y:%.*]], [[X:%.*]]
 ; CHECK-NEXT:    call void @use(i8 [[S2]])
-; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.abs.i8(i8 [[S1]], i1 false)
+; CHECK-NEXT:    [[TMP1:%.*]] = call i8 @llvm.abs.i8(i8 [[S2]], i1 false)
 ; CHECK-NEXT:    ret i8 [[TMP1]]
 ;
   %s1 = sub i8 %x, %y