-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[SimplifyLibCalls] Recognize and simplify f[min/max]imumnum #170699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: None (valadaptive) ChangesUnlike fmin and fmax, these are deterministic with regards to signed Full diff: https://github.com/llvm/llvm-project/pull/170699.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
index 4e7c97194cc59..64d2512308935 100644
--- a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
+++ b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
@@ -205,6 +205,7 @@ class LibCallSimplifier {
Value *replacePowWithSqrt(CallInst *Pow, IRBuilderBase &B);
Value *optimizeExp2(CallInst *CI, IRBuilderBase &B);
Value *optimizeFMinFMax(CallInst *CI, IRBuilderBase &B);
+ Value *optimizeFMinimumnumFMaximumnum(CallInst *CI, IRBuilderBase &B);
Value *optimizeLog(CallInst *CI, IRBuilderBase &B);
Value *optimizeSqrt(CallInst *CI, IRBuilderBase &B);
Value *optimizeFMod(CallInst *CI, IRBuilderBase &B);
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index d1548694baa27..20d9b03bd87c8 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -2543,6 +2543,30 @@ Value *LibCallSimplifier::optimizeFMinFMax(CallInst *CI, IRBuilderBase &B) {
CI->getArgOperand(1), FMF));
}
+Value *LibCallSimplifier::optimizeFMinimumnumFMaximumnum(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 == "fminimum_num" || Name == "fmaximum_num") &&
+ hasFloatVersion(M, Name))
+ if (Value *Ret = optimizeBinaryDoubleFP(CI, B, TLI))
+ return Ret;
+
+ // The new fminimum_num/fmaximum_num functions, unlike fmin/fmax, *are*
+ // sensitive to the sigh of zero, so we don't change the fast-math flags like
+ // we did for those.
+
+ Intrinsic::ID IID = Callee->getName().starts_with("fminimum_num")
+ ? Intrinsic::minimumnum
+ : Intrinsic::maximumnum;
+ return copyFlags(*CI, B.CreateBinaryIntrinsic(IID, CI->getArgOperand(0),
+ CI->getArgOperand(1), CI));
+}
+
Value *LibCallSimplifier::optimizeLog(CallInst *Log, IRBuilderBase &B) {
Function *LogFn = Log->getCalledFunction();
StringRef LogNm = LogFn->getName();
@@ -4123,6 +4147,13 @@ Value *LibCallSimplifier::optimizeFloatingPointLibCall(CallInst *CI,
case LibFunc_fmax:
case LibFunc_fmaxl:
return optimizeFMinFMax(CI, Builder);
+ case LibFunc_fminimum_numf:
+ case LibFunc_fminimum_num:
+ case LibFunc_fminimum_numl:
+ case LibFunc_fmaximum_numf:
+ case LibFunc_fmaximum_num:
+ case LibFunc_fmaximum_numl:
+ return optimizeFMinimumnumFMaximumnum(CI, Builder);
case LibFunc_cabs:
case LibFunc_cabsf:
case LibFunc_cabsl:
|
dtcxzyw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing tests
Unlike fmin and fmax, these are deterministic with regards to signed zero.
0f7fa39 to
2057460
Compare
|
I added tests in #170695; I've pulled those in now |
dtcxzyw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| // function, do that first. | ||
| Function *Callee = CI->getCalledFunction(); | ||
| StringRef Name = Callee->getName(); | ||
| if ((Name == "fminimum_num" || Name == "fmaximum_num") && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this need the string compare? TargetLibraryInfo should have already parsed this into an enum LibFunc in the caller? Pass that down?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mostly just adapted this from optimizeFMinFMax above. We could indeed pass in the LibFunc to both functions, although we still need to get the function name to pass into hasFloatVersion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should do that then? I would like to eliminate all string-based libcall logic throughout llvm
Unlike fmin and fmax, these are deterministic with regards to signed
zero.