Skip to content
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

[InstCombine] Handle more even/odd math functions #81324

Merged
merged 9 commits into from
Feb 24, 2024

Conversation

agentcooper
Copy link
Contributor

At the moment PR adds support only for erf function.

Does it make sense to create a list of known even/odd functions and call the newly introduced LibCallSimplifier::optimizeSymmetric at the point where optimizeTrigReflections is now called?

Fixes #77220.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 9, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Artem Tyurin (agentcooper)

Changes

At the moment PR adds support only for erf function.

Does it make sense to create a list of known even/odd functions and call the newly introduced LibCallSimplifier::optimizeSymmetric at the point where optimizeTrigReflections is now called?

Fixes #77220.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/TargetLibraryInfo.def (+15)
  • (modified) llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h (+1)
  • (modified) llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp (+19)
  • (added) llvm/test/Transforms/InstCombine/math-odd-even-parity.ll (+17)
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.def b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
index 6bd922eed89e15..a2942b538fad77 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.def
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.def
@@ -1837,6 +1837,21 @@ TLI_DEFINE_ENUM_INTERNAL(pow)
 TLI_DEFINE_STRING_INTERNAL("pow")
 TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl, Dbl)
 
+/// double erf(double x);
+TLI_DEFINE_ENUM_INTERNAL(erf)
+TLI_DEFINE_STRING_INTERNAL("erf")
+TLI_DEFINE_SIG_INTERNAL(Dbl, Dbl)
+
+/// float erf(float x);
+TLI_DEFINE_ENUM_INTERNAL(erff)
+TLI_DEFINE_STRING_INTERNAL("erff")
+TLI_DEFINE_SIG_INTERNAL(Flt, Flt)
+
+/// long double cbrtl(long double x);
+TLI_DEFINE_ENUM_INTERNAL(erfl)
+TLI_DEFINE_STRING_INTERNAL("erfl")
+TLI_DEFINE_SIG_INTERNAL(LDbl, LDbl)
+
 /// float powf(float x, float y);
 TLI_DEFINE_ENUM_INTERNAL(powf)
 TLI_DEFINE_STRING_INTERNAL("powf")
diff --git a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
index 1b6b525b19caef..05a788e4ea30f4 100644
--- a/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
+++ b/llvm/include/llvm/Transforms/Utils/SimplifyLibCalls.h
@@ -204,6 +204,7 @@ class LibCallSimplifier {
   Value *mergeSqrtToExp(CallInst *CI, IRBuilderBase &B);
   Value *optimizeSinCosPi(CallInst *CI, bool IsSin, IRBuilderBase &B);
   Value *optimizeTrigInversionPairs(CallInst *CI, IRBuilderBase &B);
+  Value *optimizeSymmetric(CallInst *CI, bool isEven, IRBuilderBase &B);
   // Wrapper for all floating point library call optimizations
   Value *optimizeFloatingPointLibCall(CallInst *CI, LibFunc Func,
                                       IRBuilderBase &B);
diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
index 26a34aa99e1b87..12551dc8f2befc 100644
--- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -2797,6 +2797,21 @@ static bool insertSinCosCall(IRBuilderBase &B, Function *OrigCallee, Value *Arg,
   return true;
 }
 
+Value *LibCallSimplifier::optimizeSymmetric(CallInst *CI, bool IsEven,
+                                            IRBuilderBase &B) {
+  Value *X;
+  if (match(CI->getArgOperand(0), m_OneUse(m_FNeg(m_Value(X))))) {
+    auto *CallInst = copyFlags(*CI, B.CreateCall(CI->getCalledFunction(), {X}));
+    if (IsEven) {
+      // Even function: f(-x) = f(x)
+      return CallInst;
+    }
+    // Odd function: f(-x) = -f(x)
+    return B.CreateFNeg(CallInst);
+  }
+  return nullptr;
+}
+
 Value *LibCallSimplifier::optimizeSinCosPi(CallInst *CI, bool IsSin, IRBuilderBase &B) {
   // Make sure the prototype is as expected, otherwise the rest of the
   // function is probably invalid and likely to abort.
@@ -3779,6 +3794,10 @@ Value *LibCallSimplifier::optimizeFloatingPointLibCall(CallInst *CI,
   case LibFunc_cabsf:
   case LibFunc_cabsl:
     return optimizeCAbs(CI, Builder);
+  case LibFunc_erf:
+  case LibFunc_erff:
+  case LibFunc_erfl:
+    return optimizeSymmetric(CI, /*IsEven*/ false, Builder);
   default:
     return nullptr;
   }
diff --git a/llvm/test/Transforms/InstCombine/math-odd-even-parity.ll b/llvm/test/Transforms/InstCombine/math-odd-even-parity.ll
new file mode 100644
index 00000000000000..2372ff3c97966b
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/math-odd-even-parity.ll
@@ -0,0 +1,17 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+declare double @erf(double)
+
+; Check odd parity: -erf(-x) == erf(x)
+define double @test_erf1(double %x) {
+; CHECK-LABEL: define double @test_erf1(
+; CHECK-SAME: double [[X:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @erf(double [[X]])
+; CHECK-NEXT:    ret double [[TMP1]]
+;
+  %neg_x = fneg double %x
+  %res = call double @erf(double %neg_x)
+  %neg_res = fneg double %res
+  ret double %neg_res
+}

llvm/include/llvm/Analysis/TargetLibraryInfo.def Outdated Show resolved Hide resolved
llvm/include/llvm/Analysis/TargetLibraryInfo.def Outdated Show resolved Hide resolved
llvm/include/llvm/Analysis/TargetLibraryInfo.def Outdated Show resolved Hide resolved
llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Feb 10, 2024

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

@agentcooper
Copy link
Contributor Author

@pogo59 Could you please help with updating llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml? It is unclear to me how it should be updated when adding new math.h functions.

@Krishna-13-cyber
Copy link
Contributor

Krishna-13-cyber commented Feb 10, 2024

@pogo59 Could you please help with updating llvm/test/tools/llvm-tli-checker/ps4-tli-check.yaml? It is unclear to me how it should be updated when adding new math.h functions.

Hi @agentcooper,
You can just set the added functions as unavailable for PS4/PS5 in TargetLibraryInfo.cpp which will help you pass all the checks.

    +TLI.setUnavailable(LibFunc_erf);
    +TLI.setUnavailable(LibFunc_erff);
    +TLI.setUnavailable(LibFunc_erfl);

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!
Please wait for additional approval from other reviewers.

case LibFunc_cos:
case LibFunc_cosf:
case LibFunc_cosl: {
// cos(-x) --> cos(x)
// cos(fabs(x)) --> cos(x)
Copy link
Member

Choose a reason for hiding this comment

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

I think f(fabs(x)) --> f(x) and f(copysign(x, y)) --> f(x) also hold for even functions.

@pogo59
Copy link
Collaborator

pogo59 commented Feb 12, 2024

You can just set the added functions as unavailable for PS4/PS5 in TargetLibraryInfo.cpp which will help you pass all the checks.

That will work for now. I'll add an internal task to handle these properly on our side. Thanks!

@agentcooper
Copy link
Contributor Author

@arsenm @jcranmer-intel Could you please take a look?

@agentcooper
Copy link
Contributor Author

agentcooper commented Feb 24, 2024

@jcranmer-intel @dtcxzyw Could you please land this? I don't have commit access. The Linux build is green on CI and failure in the Windows build seems to be unrelated to this PR.

@dtcxzyw
Copy link
Member

dtcxzyw commented Feb 24, 2024

Thanks for your contribution!

@dtcxzyw dtcxzyw merged commit 1901f44 into llvm:main Feb 24, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle f(-x)=f(x) for even functions and f(-x)=-f(x) for odd functions
6 participants