[InstCombine] Remove m_OneUse requirement for max, but not min

[AZero13]

If it is ever determined that min doesn't need one-use, then we can remove the one-use requirement entirely.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

If it is ever determined that min doesn't need one-use, then we can in fact remove that version entirely.

---

Full diff: https://github.com/llvm/llvm-project/pull/81505.diff

3 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+21-6)
• (modified) llvm/test/Transforms/InstCombine/maximum.ll (+1-1)
• (modified) llvm/test/Transforms/InstCombine/maxnum.ll (+1-1)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 56d1259e955196..4525f4e5c45832 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -2265,14 +2265,29 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
 
     // max X, -X --> fabs X
     // min X, -X --> -(fabs X)
-    // TODO: Remove one-use limitation? That is obviously better for max.
-    //       It would be an extra instruction for min (fnabs), but that is
-    //       still likely better for analysis and codegen.
-    if ((match(Arg0, m_OneUse(m_FNeg(m_Value(X)))) && Arg1 == X) ||
-        (match(Arg1, m_OneUse(m_FNeg(m_Value(X)))) && Arg0 == X)) {
+
+    if ((match(Arg0, m_OneUse(m_FNeg(m_Value(X)))) &&
+         match(Arg1, m_Specific(X))) ||
+        (match(Arg1, m_OneUse(m_FNeg(m_Value(X)))) &&
+         match(Arg0, m_Specific(X)))) {
       Value *R = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, II);
-      if (IID == Intrinsic::minimum || IID == Intrinsic::minnum)
+      if (IID == Intrinsic::minimum || IID == Intrinsic::minnum) {
         R = Builder.CreateFNegFMF(R, II);
+      }
+
+      return replaceInstUsesWith(*II, R);
+    }
+
+    // No one-use. Only for max.
+    // TODO: Remove one-use limitation? That is obviously better for max,
+    // hence why we don't check for one-use for that. However,
+    // it would be an extra instruction for min (fnabs), but
+    // that is still likely better for analysis and codegen. If so, delete
+    // one-use version
+    if ((((match(Arg0, m_FNeg(m_Value(X)))) && match(Arg1, m_Specific(X))) ||
+         (match(Arg1, m_FNeg(m_Value(X))) && match(Arg0, m_Specific(X)))) &&
+        IID != Intrinsic::minimum && IID != Intrinsic::minnum) {
+      Value *R = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, II);
       return replaceInstUsesWith(*II, R);
     }
 
diff --git a/llvm/test/Transforms/InstCombine/maximum.ll b/llvm/test/Transforms/InstCombine/maximum.ll
index 82e4c8794c1c84..88cb2984510002 100644
--- a/llvm/test/Transforms/InstCombine/maximum.ll
+++ b/llvm/test/Transforms/InstCombine/maximum.ll
@@ -436,7 +436,7 @@ define float @negated_op_extra_use(float %x) {
 ; CHECK-LABEL: @negated_op_extra_use(
 ; CHECK-NEXT:    [[NEGX:%.*]] = fneg float [[X:%.*]]
 ; CHECK-NEXT:    call void @use(float [[NEGX]])
-; CHECK-NEXT:    [[R:%.*]] = call float @llvm.maximum.f32(float [[NEGX]], float [[X]])
+; CHECK-NEXT:    [[R:%.*]] = call float @llvm.fabs.f32(float [[X]])
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %negx = fneg float %x
diff --git a/llvm/test/Transforms/InstCombine/maxnum.ll b/llvm/test/Transforms/InstCombine/maxnum.ll
index 87288b18cbcd9f..a1a2b096cb2745 100644
--- a/llvm/test/Transforms/InstCombine/maxnum.ll
+++ b/llvm/test/Transforms/InstCombine/maxnum.ll
@@ -458,7 +458,7 @@ define float @negated_op_extra_use(float %x) {
 ; CHECK-LABEL: @negated_op_extra_use(
 ; CHECK-NEXT:    [[NEGX:%.*]] = fneg float [[X:%.*]]
 ; CHECK-NEXT:    call void @use(float [[NEGX]])
-; CHECK-NEXT:    [[R:%.*]] = call float @llvm.maxnum.f32(float [[NEGX]], float [[X]])
+; CHECK-NEXT:    [[R:%.*]] = call float @llvm.fabs.f32(float [[X]])
 ; CHECK-NEXT:    ret float [[R]]
 ;
   %negx = fneg float %x

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[arsenm]

Not enough test diffs; the commuted cases aren't being tested

[AZero13]

Not enough test diffs; the commuted cases aren't being tested

@arsenm Addressed!

[AZero13]

@arsenm I do not have commit permissions.