[SimplifyLibCalls] Directly convert fmin/fmax to intrinsics - #177988
Merged
Conversation
llvmbot
added
llvm:instcombine
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesDrop the custom shrinking code, which we'll also do for intrinsics. Having libcall-only optimizations is confusing, as these are typically directly emitted as intrinsics by the frontend. However, the fold on intrinsics is limited to one-use fpexts, so the behavior is not completely the same: llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp Lines 2870 to 2877 in d7666c6 I think we could relax that condition to only one of the fpexts being one-use. Let me know if you want me to do that. Full diff: https://github.com/llvm/llvm-project/pull/177988.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
index 64d2512308935..9ba3455254f47 100644
--- a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
+++ b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
@@ -204,7 +204,7 @@ class LibCallSimplifier {
Value *replacePowWithExp(CallInst *Pow, IRBuilderBase &B);
Value *replacePowWithSqrt(CallInst *Pow, IRBuilderBase &B);
Value *optimizeExp2(CallInst *CI, IRBuilderBase &B);
- Value *optimizeFMinFMax(CallInst *CI, IRBuilderBase &B);
+ Value *optimizeFMinFMax(CallInst *CI, IRBuilderBase &B, Intrinsic::ID IID);
Value *optimizeFMinimumnumFMaximumnum(CallInst *CI, IRBuilderBase &B);
Value *optimizeLog(CallInst *CI, IRBuilderBase &B);
Value *optimizeSqrt(CallInst *CI, IRBuilderBase &B);
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index 1e8a184fc51b8..ff05faf1c2200 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -2520,17 +2520,8 @@ Value *LibCallSimplifier::optimizeExp2(CallInst *CI, IRBuilderBase &B) {
return Ret;
}
-Value *LibCallSimplifier::optimizeFMinFMax(CallInst *CI, IRBuilderBase &B) {
- Module *M = CI->getModule();
-
- // If we can shrink the call to a float function rather than a double
- // function, do that first.
- Function *Callee = CI->getCalledFunction();
- StringRef Name = Callee->getName();
- if ((Name == "fmin" || Name == "fmax") && hasFloatVersion(M, Name))
- if (Value *Ret = optimizeBinaryDoubleFP(CI, B, TLI))
- return Ret;
-
+Value *LibCallSimplifier::optimizeFMinFMax(CallInst *CI, IRBuilderBase &B,
+ Intrinsic::ID IID) {
// The LLVM intrinsics minnum/maxnum correspond to fmin/fmax. Canonicalize to
// the intrinsics for improved optimization (for example, vectorization).
// No-signed-zeros is implied by the definitions of fmax/fmin themselves.
@@ -2540,9 +2531,6 @@ Value *LibCallSimplifier::optimizeFMinFMax(CallInst *CI, IRBuilderBase &B) {
// might be impractical."
FastMathFlags FMF = CI->getFastMathFlags();
FMF.setNoSignedZeros();
-
- Intrinsic::ID IID = Callee->getName().starts_with("fmin") ? Intrinsic::minnum
- : Intrinsic::maxnum;
return copyFlags(*CI, B.CreateBinaryIntrinsic(IID, CI->getArgOperand(0),
CI->getArgOperand(1), FMF));
}
@@ -4147,10 +4135,11 @@ Value *LibCallSimplifier::optimizeFloatingPointLibCall(CallInst *CI,
case LibFunc_fminf:
case LibFunc_fmin:
case LibFunc_fminl:
+ return optimizeFMinFMax(CI, Builder, Intrinsic::minnum);
case LibFunc_fmaxf:
case LibFunc_fmax:
case LibFunc_fmaxl:
- return optimizeFMinFMax(CI, Builder);
+ return optimizeFMinFMax(CI, Builder, Intrinsic::maxnum);
case LibFunc_fminimum_numf:
case LibFunc_fminimum_num:
case LibFunc_fminimum_numl:
diff --git a/llvm/test/Transforms/InstCombine/float-shrink-compare.ll b/llvm/test/Transforms/InstCombine/float-shrink-compare.ll
index 6383feff3a6ee..452df5118a324 100644
--- a/llvm/test/Transforms/InstCombine/float-shrink-compare.ll
+++ b/llvm/test/Transforms/InstCombine/float-shrink-compare.ll
@@ -507,8 +507,10 @@ define i1 @test19(float %x, float %y, float %z) {
define i1 @test20(float %x, float %y) {
; CHECK-LABEL: @test20(
-; CHECK-NEXT: [[FMINF:%.*]] = call nsz float @llvm.minnum.f32(float [[X:%.*]], float 1.000000e+00)
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp oeq float [[Y:%.*]], [[FMINF]]
+; CHECK-NEXT: [[TMP4:%.*]] = fpext float [[Y:%.*]] to double
+; CHECK-NEXT: [[TMP2:%.*]] = fpext float [[X:%.*]] to double
+; CHECK-NEXT: [[TMP3:%.*]] = call nsz double @llvm.minnum.f64(double [[TMP2]], double 1.000000e+00)
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp oeq double [[TMP3]], [[TMP4]]
; CHECK-NEXT: ret i1 [[TMP1]]
;
%1 = fpext float %y to double
|
🪟 Windows x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.CodeGen/AMDGPU/kernel-args.llIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.CodeGen/AMDGPU/kernel-args.llIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
dtcxzyw
approved these changes
Jan 26, 2026
dtcxzyw
left a comment
dtcxzyw
left a comment
Member
There was a problem hiding this comment.
I think we could relax that condition to only one of the fpexts being one-use. Let me know if you want me to do that.
Make sense to me. But relaxing the one-use constraint doesn't restore the test change. Note that the LHS is a constant, and valueHasFloatPrecision is responsible for converting it to float losslessly.
llvm-project/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
Lines 1956 to 1963 in 6233946
Contributor
Author
Yeah, not sure what I was looking at when I wrote this... I've now opened both #179968 (for the constant case) and #180164 (for the multi-use case), both of which are profitable on llvm-opt-benchmark. |
nikic
added a commit
that referenced
this pull request
Feb 9, 2026
#179968) Fold `min/max(fpext x, C)` to `fpext(min/max(x, fptrunc C))` in cases where the truncation of the constant is lossless. This helps eliminate fpext/fptrunc pairs around min/max and addresses the regression from #177988. Proof: https://alive2.llvm.org/ce/z/y_Bcdd
Drop the custom shrinking code, which we'll also do for intrinsics. Having libcall-only optimizations is confusing, as these are typically directly emitted as intrinsics by the frontend.
nikic
enabled auto-merge (squash)
February 9, 2026 09:17
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 9, 2026
… fptrunc C)) (#179968) Fold `min/max(fpext x, C)` to `fpext(min/max(x, fptrunc C))` in cases where the truncation of the constant is lossless. This helps eliminate fpext/fptrunc pairs around min/max and addresses the regression from llvm/llvm-project#177988. Proof: https://alive2.llvm.org/ce/z/y_Bcdd
rishabhmadan19
pushed a commit
to rishabhmadan19/llvm-project
that referenced
this pull request
Feb 9, 2026
llvm#179968) Fold `min/max(fpext x, C)` to `fpext(min/max(x, fptrunc C))` in cases where the truncation of the constant is lossless. This helps eliminate fpext/fptrunc pairs around min/max and addresses the regression from llvm#177988. Proof: https://alive2.llvm.org/ce/z/y_Bcdd
rishabhmadan19
pushed a commit
to rishabhmadan19/llvm-project
that referenced
this pull request
Feb 9, 2026
llvm-sync Bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 10, 2026
…ntrinsic (#180555) Same as llvm/llvm-project#177988, but for fminimum_num/fmaximum_num. Directly canonicalize these to the corresponding intrinsics, and let the shrinking happen directly on the intrinsics.
markrvmurray
pushed a commit
to markrvmurray/llvm-mc6809
that referenced
this pull request
Jun 14, 2026
…) (#179968) Fold `min/max(fpext x, C)` to `fpext(min/max(x, fptrunc C))` in cases where the truncation of the constant is lossless. This helps eliminate fpext/fptrunc pairs around min/max and addresses the regression from llvm/llvm-project#177988. Proof: https://alive2.llvm.org/ce/z/y_Bcdd
markrvmurray
pushed a commit
to markrvmurray/llvm-mc6809
that referenced
this pull request
Jun 14, 2026
…180555) Same as llvm/llvm-project#177988, but for fminimum_num/fmaximum_num. Directly canonicalize these to the corresponding intrinsics, and let the shrinking happen directly on the intrinsics.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drop the custom shrinking code, which we'll also do for intrinsics. Having libcall-only optimizations is confusing, as these are typically directly emitted as intrinsics by the frontend.