Skip to content

[TTI] Treat *l (long double) and *f128 (_Float128) libcalls the same #148311

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

As noted in the preexisting FIXME above this hunk, the logic in this function is not always accurate. However, the logic for long double and _Float128 should be approximately the same for these functions (almost everything requiring a libcall), so it makes sense to treat them the same.

This is meant to reduce churn in cases where *l and *f128 lowerings call to the same symbol so can be used interchangeably.

Copy link

github-actions bot commented Jul 11, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

As noted in the preexisting FIXME above this hunk, the logic in this
function is not always accurate. However, the logic for `long double`
and `_Float128` should be approximately the same for these functions
(almost everything requiring a libcall), so it makes sense to treat them
the same.

This is meant to reduce churn in cases where `*l` and `*f128` lowerings
call to the same symbol so can be used interchangeably.
@tgross35 tgross35 marked this pull request as ready for review July 11, 2025 22:55
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Jul 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 11, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Trevor Gross (tgross35)

Changes

As noted in the preexisting FIXME above this hunk, the logic in this function is not always accurate. However, the logic for long double and _Float128 should be approximately the same for these functions (almost everything requiring a libcall), so it makes sense to treat them the same.

This is meant to reduce churn in cases where *l and *f128 lowerings call to the same symbol so can be used interchangeably.


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

1 Files Affected:

  • (modified) llvm/include/llvm/Analysis/TargetTransformInfoImpl.h (+19-18)
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index ddc8a5eaffa94..bae499d99b092 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -189,27 +189,28 @@ class TargetTransformInfoImplBase {
 
     // These will all likely lower to a single selection DAG node.
     // clang-format off
-    if (Name == "copysign" || Name == "copysignf" || Name == "copysignl" ||
-        Name == "fabs"  || Name == "fabsf"  || Name == "fabsl" ||
-        Name == "fmin"  || Name == "fminf"  || Name == "fminl" ||
-        Name == "fmax"  || Name == "fmaxf"  || Name == "fmaxl" ||
-        Name == "sin"   || Name == "sinf"   || Name == "sinl"  ||
-        Name == "cos"   || Name == "cosf"   || Name == "cosl"  ||
-        Name == "tan"   || Name == "tanf"   || Name == "tanl"  ||
-        Name == "asin"  || Name == "asinf"  || Name == "asinl" ||
-        Name == "acos"  || Name == "acosf"  || Name == "acosl" ||
-        Name == "atan"  || Name == "atanf"  || Name == "atanl" ||
-        Name == "atan2" || Name == "atan2f" || Name == "atan2l"||
-        Name == "sinh"  || Name == "sinhf"  || Name == "sinhl" ||
-        Name == "cosh"  || Name == "coshf"  || Name == "coshl" ||
-        Name == "tanh"  || Name == "tanhf"  || Name == "tanhl" ||
-        Name == "sqrt"  || Name == "sqrtf"  || Name == "sqrtl" ||
-        Name == "exp10"  || Name == "exp10l"  || Name == "exp10f")
+    if (Name == "copysign" || Name == "copysignf" || Name == "copysignl" || Name == "copysignl128" ||
+        Name == "fabs"  || Name == "fabsf"  || Name == "fabsl" || Name == "fabsl128" ||
+        Name == "fmin"  || Name == "fminf"  || Name == "fminl" || Name == "fminl128" ||
+        Name == "fmax"  || Name == "fmaxf"  || Name == "fmaxl" || Name == "fmaxl128" ||
+        Name == "sin"   || Name == "sinf"   || Name == "sinl"  || Name == "sinl128"  ||
+        Name == "cos"   || Name == "cosf"   || Name == "cosl"  || Name == "cosl128"  ||
+        Name == "tan"   || Name == "tanf"   || Name == "tanl"  || Name == "tanl128"  ||
+        Name == "asin"  || Name == "asinf"  || Name == "asinl" || Name == "asinl128" ||
+        Name == "acos"  || Name == "acosf"  || Name == "acosl" || Name == "acosl128" ||
+        Name == "atan"  || Name == "atanf"  || Name == "atanl" || Name == "atanl128" ||
+        Name == "atan2" || Name == "atan2f" || Name == "atan2l"|| Name == "atan2128l"||
+        Name == "sinh"  || Name == "sinhf"  || Name == "sinhl" || Name == "sinhl128" ||
+        Name == "cosh"  || Name == "coshf"  || Name == "coshl" || Name == "coshl128" ||
+        Name == "tanh"  || Name == "tanhf"  || Name == "tanhl" || Name == "tanhl128" ||
+        Name == "sqrt"  || Name == "sqrtf"  || Name == "sqrtl" || Name == "sqrtl128" ||
+        Name == "exp10" || Name == "exp10f" || Name == "exp10l"|| Name == "exp10f128")
       return false;
     // clang-format on
     // These are all likely to be optimized into something smaller.
-    if (Name == "pow" || Name == "powf" || Name == "powl" || Name == "exp2" ||
-        Name == "exp2l" || Name == "exp2f" || Name == "floor" ||
+    if (Name == "pow" || Name == "powf" || Name == "powl" ||
+        Name == "powf128" || Name == "exp2" || Name == "exp2f" ||
+        Name == "exp2l" || Name == "powf128" || Name == "floor" ||
         Name == "floorf" || Name == "ceil" || Name == "round" ||
         Name == "ffs" || Name == "ffsl" || Name == "abs" || Name == "labs" ||
         Name == "llabs")

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be tested?

@nikic nikic requested a review from arsenm July 12, 2025 10:03
@tgross35
Copy link
Contributor Author

Can this be tested?

I assume there is a test for the current *l behavior that could be expanded, but I'm not sure what this would look like - any idea where to look?

@nikic
Copy link
Contributor

nikic commented Jul 12, 2025

Can this be tested?

I assume there is a test for the current *l behavior that could be expanded, but I'm not sure what this would look like - any idea where to look?

Comment the code out, run the tests, and see what fails :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants