ConstantFold logl calls #94944

MDevereau · 2024-06-10T09:23:07Z

This is a follow up patch from #90611 which folds logl calls in the same manner as log.f128 calls. logl suffers from the same problem as logf128 of having slow calls to fp128 log functions which can be constant folded. However, logl is emitted with -fmath-errno and log.f128 is emitted by -fno-math-errno by certain intrinsics.

This is a follow up patch from llvm#90611 which folds logl calls in the same manner as log.f128 calls. Logl suffers from the same problem as logf128 of having slow calls to fp128 log functions which can be constant folded. However, Logl is emitted at the O3 level instead whereas log.f128 is emitted by Ofast by various intrinsics.

llvmbot · 2024-06-10T09:23:37Z

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Matthew Devereau (MDevereau)

Changes

This is a follow up patch from #90611 which folds logl calls in the same manner as log.f128 calls. Logl suffers from the same problem as logf128 of having slow calls to fp128 log functions which can be constant folded. However, Logl is emitted at the O3 level instead whereas log.f128 is emitted by Ofast by various intrinsics.

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

2 Files Affected:

(modified) llvm/lib/Analysis/ConstantFolding.cpp (+15-9)
(modified) llvm/test/Transforms/InstSimplify/ConstProp/logf128.ll (+10)

diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index 3ca3ae951fcd7..c0b9f7a4d68da 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -1669,9 +1669,9 @@ bool llvm::canConstantFoldCallTo(const CallBase *Call, const Function *F) {
            Name == "floor" || Name == "floorf" ||
            Name == "fmod" || Name == "fmodf";
   case 'l':
-    return Name == "log" || Name == "logf" ||
-           Name == "log2" || Name == "log2f" ||
-           Name == "log10" || Name == "log10f";
+    return Name == "log" || Name == "logf" || Name == "log2" ||
+           Name == "log2f" || Name == "log10" || Name == "log10f" ||
+           Name == "logl";
   case 'n':
     return Name == "nearbyint" || Name == "nearbyintf";
   case 'p':
@@ -2085,15 +2085,19 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
     if (IntrinsicID == Intrinsic::canonicalize)
       return constantFoldCanonicalize(Ty, Call, U);
 
+      // Try to handle special fp128 cases before bailing
 #if defined(HAS_IEE754_FLOAT128) && defined(HAS_LOGF128)
     if (Ty->isFP128Ty()) {
-      switch (IntrinsicID) {
-      default:
-        return nullptr;
-      case Intrinsic::log:
-        return ConstantFP::get(Ty, logf128(Op->getValueAPF().convertToQuad()));
-      }
+      const APFloat &Fp128APF = Op->getValueAPF();
+      if (IntrinsicID == Intrinsic::log)
+        return ConstantFP::get(Ty, logf128(Fp128APF.convertToQuad()));
     }
+
+    LibFunc Fp128Func = NotLibFunc;
+    if (Ty->isFP128Ty() && TLI->getLibFunc(Name, Fp128Func) &&
+        TLI->has(Fp128Func) && Fp128Func == LibFunc_logl &&
+        !Op->getValueAPF().isNegative() && !Op->getValueAPF().isZero())
+      return ConstantFP::get(Ty, logf128(Op->getValueAPF().convertToQuad()));
 #endif
 
     if (!Ty->isHalfTy() && !Ty->isFloatTy() && !Ty->isDoubleTy())
@@ -2356,6 +2360,8 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
         // TODO: What about hosts that lack a C99 library?
         return ConstantFoldFP(log10, APF, Ty);
       break;
+    case LibFunc_logl:
+      return nullptr;
     case LibFunc_nearbyint:
     case LibFunc_nearbyintf:
     case LibFunc_rint:
diff --git a/llvm/test/Transforms/InstSimplify/ConstProp/logf128.ll b/llvm/test/Transforms/InstSimplify/ConstProp/logf128.ll
index da56997f69382..051d514058ccc 100644
--- a/llvm/test/Transforms/InstSimplify/ConstProp/logf128.ll
+++ b/llvm/test/Transforms/InstSimplify/ConstProp/logf128.ll
@@ -3,6 +3,7 @@
 
 ; REQUIRES: has_logf128
 declare fp128 @llvm.log.f128(fp128)
+declare fp128 @logl(fp128)
 
 define fp128 @log_e_64(){
 ; CHECK-LABEL: define fp128 @log_e_64() {
@@ -124,3 +125,12 @@ define <2 x fp128> @log_e_negative_2_vector(){
   %A = call <2 x fp128> @llvm.log.v2f128(<2 x fp128> <fp128 0xL0000000000000000C000000000000000, fp128 0xL0000000000000000C000000000000001>)
   ret <2 x fp128> %A
 }
+
+define fp128 @logl_e_64(){
+; CHECK-LABEL: define fp128 @logl_e_64() {
+; CHECK-NEXT:    %A = call fp128 @logl(fp128 noundef 0xL00000000000000004005000000000000)
+; CHECK-NEXT:    ret fp128 0xL300000000000000040010A2B23F3BAB7
+;
+  %A = call fp128 @logl(fp128 noundef 0xL00000000000000004005000000000000)
+  ret fp128 %A
+}

arsenm · 2024-06-10T09:25:07Z

Description is imprecise, intrinsic or libcall depends on using -fmath-errno/-fno-math-errno only. Don't mention -Ofast

MDevereau · 2024-06-11T11:47:25Z

Description is imprecise, intrinsic or libcall depends on using -fmath-errno/-fno-math-errno only. Don't mention -Ofast

I've removed -Ofast/-O3 references, thanks.

davemgreen

Could you add tests with out-of-range values? 0 and inf and nan and negatives?

MDevereau · 2024-06-12T08:13:27Z

Could you add tests with out-of-range values? 0 and inf and nan and negatives?

Done, I've removed the zero and negative checks as well to keep it consistent with the logf.128 behaviour.

davemgreen · 2024-06-12T08:25:39Z

Done, I've removed the zero and negative checks as well to keep it consistent with the logf.128 behaviour.

It needs to produce the same errno and fpexeption values. I can see that the intrinsic remains, but would recommend making it work the same as floats: https://godbolt.org/z/T5x7foe4Y

arsenm · 2024-06-12T08:54:09Z

llvm/lib/Analysis/ConstantFolding.cpp

    if (Ty->isFP128Ty()) {
-      switch (IntrinsicID) {
-      default:
-        return nullptr;
-      case Intrinsic::log:
-        return ConstantFP::get(Ty, logf128(Op->getValueAPF().convertToQuad()));
-      }
+      const APFloat &Fp128APF = Op->getValueAPF();
+      if (IntrinsicID == Intrinsic::log)
+        return ConstantFP::get(Ty, logf128(Fp128APF.convertToQuad()));
    }
+
+    LibFunc Fp128Func = NotLibFunc;
+    if (Ty->isFP128Ty() && TLI->getLibFunc(Name, Fp128Func) &&
+        TLI->has(Fp128Func) && Fp128Func == LibFunc_logl)
+      return ConstantFP::get(Ty, logf128(Op->getValueAPF().convertToQuad()));


Combine these under 1 isFP128Ty check?

MDevereau · 2024-06-14T07:37:15Z

Done, I've removed the zero and negative checks as well to keep it consistent with the logf.128 behaviour.

It needs to produce the same errno and fpexeption values. I can see that the intrinsic remains, but would recommend making it work the same as floats: https://godbolt.org/z/T5x7foe4Y

I think I've done what you asked for. I'm a bit unsure about the infinity test case though.

davemgreen

As far as I understand the difference between logl and llvm.log.f128 is that logl needs to set errno in the right places and has side-effects, llvm.log.f128 doesn't require that. That is why we convert logl to llvm.log.f128 under Ofast/-fno-math-errno. So llvm.log.f128(0) should still be fine to be constant-folded to Nan, it is only the logl calls that need to be more careful.

MDevereau · 2024-06-14T10:32:15Z

As far as I understand the difference between logl and llvm.log.f128 is that logl needs to set errno in the right places and has side-effects, llvm.log.f128 doesn't require that. That is why we convert logl to llvm.log.f128 under Ofast/-fno-math-errno. So llvm.log.f128(0) should still be fine to be constant-folded to Nan, it is only the logl calls that need to be more careful.

Makes sense, I've removed the checks for log.f128 then. With logl infinity, I was unsure whether the fact it hasn't retained the original logl call was correct behaviour or not.

davemgreen

Makes sense, I've removed the checks for log.f128 then. With logl infinity, I was unsure whether the fact it hasn't retained the original logl call was correct behaviour or not.

It looks like it doesn't generate a pole error for infinity when I tried it (only for zero), so I think that sounds OK. From what I can tell this LGTM, thanks.

Uses of __float128 in (#94944) should be float128. Although ConstantFoldFP128 is not reliant on HAS_LOGF128, it is only used by conditional code controlled by HAS_LOGF128, and will cause unused errors on buildbots.

Uses of __float128 in (llvm#94944) should be float128. Although ConstantFoldFP128 is not reliant on HAS_LOGF128, it is only used by conditional code controlled by HAS_LOGF128, and will cause unused errors on buildbots.

Fix the build failure caused by #94944 Fixes #100296

Fix the build failure caused by llvm#94944 Fixes llvm#100296

Fix the build failure caused by llvm#94944 Fixes llvm#100296 (cherry picked from commit 40b4fd7)

MDevereau requested review from arsenm, davemgreen and david-arm June 10, 2024 09:23

llvmbot added llvm:analysis llvm:transforms labels Jun 10, 2024

davemgreen reviewed Jun 11, 2024

View reviewed changes

Remove negative and zero checks, add more tests

a0683c4

arsenm reviewed Jun 12, 2024

View reviewed changes

Run constant folded values through exception check

172f77f

davemgreen reviewed Jun 14, 2024

View reviewed changes

Remove errno check for log.f128

e49bad4

davemgreen approved these changes Jun 17, 2024

View reviewed changes

MDevereau merged commit d38c8a7 into llvm:main Jun 18, 2024
7 checks passed

MDevereau deleted the fold-logl branch June 18, 2024 12:27

programmerjake mentioned this pull request Jul 24, 2024

LLVM incorrectly assumes long double is the same type as _Float128 on ppc64le #100296

Closed

chenzheng1030 mentioned this pull request Jul 29, 2024

[NFC] fix build failure #100993

Merged

chenzheng1030 pushed a commit that referenced this pull request Jul 30, 2024

[NFC] fix build failure (#100993)

40b4fd7

Fix the build failure caused by #94944 Fixes #100296

banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024

[NFC] fix build failure (llvm#100993)

9805015

Fix the build failure caused by llvm#94944 Fixes llvm#100296

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 16, 2024

[NFC] fix build failure (llvm#100993)

f20d195

Fix the build failure caused by llvm#94944 Fixes llvm#100296 (cherry picked from commit 40b4fd7)

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Oct 29, 2024

[NFC] fix build failure (llvm#100993)

6ee4908

Fix the build failure caused by llvm#94944 Fixes llvm#100296 (cherry picked from commit 40b4fd7)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ConstantFold logl calls #94944

ConstantFold logl calls #94944

MDevereau commented Jun 10, 2024 •

edited

Loading

llvmbot commented Jun 10, 2024 •

edited

Loading

arsenm commented Jun 10, 2024

MDevereau commented Jun 11, 2024

davemgreen left a comment

MDevereau commented Jun 12, 2024

davemgreen commented Jun 12, 2024

arsenm Jun 12, 2024

MDevereau Jun 14, 2024

MDevereau commented Jun 14, 2024

davemgreen left a comment

MDevereau commented Jun 14, 2024

davemgreen left a comment

ConstantFold logl calls #94944

ConstantFold logl calls #94944

Conversation

MDevereau commented Jun 10, 2024 • edited Loading

llvmbot commented Jun 10, 2024 • edited Loading

arsenm commented Jun 10, 2024

MDevereau commented Jun 11, 2024

davemgreen left a comment

Choose a reason for hiding this comment

MDevereau commented Jun 12, 2024

davemgreen commented Jun 12, 2024

arsenm Jun 12, 2024

Choose a reason for hiding this comment

MDevereau Jun 14, 2024

Choose a reason for hiding this comment

MDevereau commented Jun 14, 2024

davemgreen left a comment

Choose a reason for hiding this comment

MDevereau commented Jun 14, 2024

davemgreen left a comment

Choose a reason for hiding this comment

MDevereau commented Jun 10, 2024 •

edited

Loading

llvmbot commented Jun 10, 2024 •

edited

Loading