Skip to content

[InstCombine] Optimize sinh and cosh divisions #81433

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 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 67 additions & 8 deletions llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2179,23 +2179,82 @@ Instruction *InstCombinerImpl::visitFDiv(BinaryOperator &I) {
Value *X;
bool IsTan = match(Op0, m_Intrinsic<Intrinsic::sin>(m_Value(X))) &&
match(Op1, m_Intrinsic<Intrinsic::cos>(m_Specific(X)));
bool IsCot =
!IsTan && match(Op0, m_Intrinsic<Intrinsic::cos>(m_Value(X))) &&
match(Op1, m_Intrinsic<Intrinsic::sin>(m_Specific(X)));
bool IsCot = !IsTan &&
match(Op0, m_Intrinsic<Intrinsic::cos>(m_Value(X))) &&
match(Op1, m_Intrinsic<Intrinsic::sin>(m_Specific(X)));

if ((IsTan || IsCot) && hasFloatFn(M, &TLI, I.getType(), LibFunc_tan,
LibFunc_tanf, LibFunc_tanl)) {
auto GetReplacement = [&](Value *Arg, bool IsInv, LibFunc DoubleFunc,
LibFunc FloatFunc,
LibFunc LongDoubleFunc) -> Value * {
IRBuilder<> B(&I);
IRBuilder<>::FastMathFlagGuard FMFGuard(B);
B.setFastMathFlags(I.getFastMathFlags());
AttributeList Attrs =
cast<CallBase>(Op0)->getCalledFunction()->getAttributes();
Value *Res = emitUnaryFloatFnCall(X, &TLI, LibFunc_tan, LibFunc_tanf,
LibFunc_tanl, B, Attrs);
if (IsCot)
Value *Res = emitUnaryFloatFnCall(Arg, &TLI, DoubleFunc, FloatFunc,
LongDoubleFunc, B, Attrs);

if (IsInv)
Res = B.CreateFDiv(ConstantFP::get(I.getType(), 1.0), Res);

return Res;
};

if ((IsTan || IsCot) && hasFloatFn(M, &TLI, I.getType(), LibFunc_tan,
LibFunc_tanf, LibFunc_tanl)) {

Value *Res =
GetReplacement(X, IsCot, LibFunc_tan, LibFunc_tanf, LibFunc_tanl);

return replaceInstUsesWith(I, Res);
}

// sinh(X) / cosh(X) -> tanh(X)
// cosh(X) / sinh(X) -> 1/tanh(X)
Value *Y;
CallBase *Op0AsCallBase = dyn_cast<CallBase>(Op0);
CallBase *Op1AsCallBase = dyn_cast<CallBase>(Op1);
LibFunc Op0LibFunc, Op1LibFunc;

if (Op1AsCallBase && Op0AsCallBase) {
TLI.getLibFunc(*Op1AsCallBase, Op1LibFunc);
TLI.getLibFunc(*Op0AsCallBase, Op0LibFunc);

bool ArgsMatch = match(Op0AsCallBase->getArgOperand(0), m_Value(Y)) &&
match(Op1AsCallBase->getArgOperand(0), m_Specific(Y));

bool IsTanH =
Copy link
Member

Choose a reason for hiding this comment

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

their are hyperbolic trig instrunctions now so you can do:

bool IsTanH = match(Op0, m_Intrinsic<Intrinsic::sinh>(m_Value(X))) &&
                 match(Op1, m_Intrinsic<Intrinsic::cosh>(m_Specific(X)));

Copy link
Contributor

Choose a reason for hiding this comment

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

But that's additive since now you have to consider the libcall and intrinsic cases (really I think these new intrinsics just increase the amount of code required to support anything)

Copy link
Member

Choose a reason for hiding this comment

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

There aren’t any LibFunc_sin or LibFunc_cos in InstCombineMulDivRem.cpp. Seems like in this case the intrinsic would result in a more simplistic ArgMatch similar to the sine and cosine cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is handling the intrinsics is not a complete solution. You can still do transforms on errno-writing libcalls, in addition to the intrinsics as long as you go from intrinsic->intrinsic and libcall->libcall. But intrinsics also suffer from another issue where there's no plausible mechanism for querying whether the intrinsic can lower, since in principle it should always be lowerable (but in the real world it's just an alias for an environment dependent libcall)

Copy link
Member

@farzonl farzonl Aug 16, 2024

Choose a reason for hiding this comment

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

I'm not disputing the correctness of your assertion. I mearly stating that there are no InstCombine cases of sine\cosine libfuncs. so doing these changes here to handle libcalls is adding a new pattern.

https://github.com/search?q=repo%3Allvm%2Fllvm-project%20LibFunc_sin&type=code
https://github.com/search?q=repo%3Allvm%2Fllvm-project+LibFunc_cos&type=code

In fact looking at the main branch the libcall pattern only exists in three other cases one of which is LibFunc_tan which I think should also be an intrinsic.

 git grep -n "LibFunc_"
InstCombineMulDivRem.cpp:1943:    if ((IsTan || IsCot) && hasFloatFn(M, &TLI, I.getType(), LibFunc_tan,
InstCombineMulDivRem.cpp:1944:                                       LibFunc_tanf, LibFunc_tanl)) {
InstCombineMulDivRem.cpp:1950:      Value *Res = emitUnaryFloatFnCall(X, &TLI, LibFunc_tan, LibFunc_tanf,
InstCombineMulDivRem.cpp:1951:                                        LibFunc_tanl, B, Attrs);
InstructionCombining.cpp:3162:            TLI.has(TheLibFunc) && TheLibFunc == LibFunc_aligned_alloc &&
InstructionCombining.cpp:3484:    if (TLI.getLibFunc(FI, Func) && TLI.has(Func) && Func == LibFunc_free

By adding LibFunc handling here we are increasing the exposure and usage of LibFunc unecessarily.

ArgsMatch &&
((Op0LibFunc == LibFunc_sinh && Op1LibFunc == LibFunc_cosh) ||
(Op0LibFunc == LibFunc_sinhf && Op1LibFunc == LibFunc_coshf) ||
(Op0LibFunc == LibFunc_sinhl && Op1LibFunc == LibFunc_coshl));

bool IsCotH =
!IsTanH && ArgsMatch &&
((Op1LibFunc == LibFunc_sinh && Op0LibFunc == LibFunc_cosh) ||
(Op1LibFunc == LibFunc_sinhf && Op0LibFunc == LibFunc_coshf) ||
(Op1LibFunc == LibFunc_sinhl && Op0LibFunc == LibFunc_coshl));

if ((IsTanH || IsCotH) && hasFloatFn(M, &TLI, I.getType(), LibFunc_tanh,
LibFunc_tanhf, LibFunc_tanhl)) {

Value *Res = GetReplacement(Y, IsCotH, LibFunc_tanh, LibFunc_tanhf,
LibFunc_tanhl);

Instruction *Replacement = replaceInstUsesWith(I, Res);

Op0AsCallBase->replaceAllUsesWith(
PoisonValue::get(Op0AsCallBase->getType()));

Op1AsCallBase->replaceAllUsesWith(
PoisonValue::get(Op1AsCallBase->getType()));

Op0AsCallBase->eraseFromParent();
Op1AsCallBase->eraseFromParent();

return Replacement;
}
}
}

// X / (X * Y) --> 1.0 / Y
Expand Down
221 changes: 221 additions & 0 deletions llvm/test/Transforms/InstCombine/fdiv-cosh-sinh.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt --mtriple "arm-apple-macosx15.0.0" -S -passes=instcombine < %s | FileCheck %s
; RUN: opt --mtriple "x86_64-apple-macosx14.7.0" -S -passes=instcombine < %s | FileCheck %s
; RUN: opt --mtriple "x86_64-microsoft-windows" -S -passes=instcombine < %s | FileCheck %s

define double @fdiv_cosh_sinh(double %a) {
; CHECK-LABEL: @fdiv_cosh_sinh(
; CHECK-NEXT: [[TMP1:%.*]] = call double @cosh(double [[A:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = call double @sinh(double [[A]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv double [[TMP1]], [[TMP2]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call double @cosh(double %a)
%sinh = call double @sinh(double %a)
%div = fdiv double %cosh, %sinh
ret double %div
}

define double @fdiv_strict_cosh_strict_sinh_reassoc(double %a) {
; CHECK-LABEL: @fdiv_strict_cosh_strict_sinh_reassoc(
; CHECK-NEXT: [[TMP1:%.*]] = call double @cosh(double [[A:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = call reassoc double @sinh(double [[A]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv double [[TMP1]], [[TMP2]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv double %cosh, %sinh
ret double %div
}

define double @fdiv_reassoc_cosh_strict_sinh_strict(double %a, ptr dereferenceable(2) %dummy) {
; CHECK-LABEL: @fdiv_reassoc_cosh_strict_sinh_strict(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call double @cosh(double %a)
%sinh = call double @sinh(double %a)
%div = fdiv reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_contract_reassoc_cosh_strict_sinh_strict(double %a, ptr dereferenceable(2) %dummy) {
; CHECK-LABEL: @fdiv_contract_reassoc_cosh_strict_sinh_strict(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc contract double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc contract double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call double @cosh(double %a)
%sinh = call double @sinh(double %a)
%div = fdiv contract reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_reassoc_cosh_reassoc_sinh_strict(double %a) {
; CHECK-LABEL: @fdiv_reassoc_cosh_reassoc_sinh_strict(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call double @sinh(double %a)
%div = fdiv reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_contract_reassoc_cosh_reassoc_sinh_strict(double %a) {
; CHECK-LABEL: @fdiv_contract_reassoc_cosh_reassoc_sinh_strict(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc contract double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc contract double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call double @sinh(double %a)
%div = fdiv contract reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_reassoc_cosh_sinh_reassoc_multiple_uses_sinh(double %a) {
; CHECK-LABEL: @fdiv_reassoc_cosh_sinh_reassoc_multiple_uses_sinh(
; CHECK-NEXT: [[COSH:%.*]] = call reassoc double @cosh(double [[A:%.*]])
; CHECK-NEXT: [[SINH:%.*]] = call reassoc double @sinh(double [[A]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc double [[COSH]], [[SINH]]
; CHECK-NEXT: call void @use(double [[SINH]])
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv reassoc double %cosh, %sinh
call void @use(double %sinh)
ret double %div
}

define double @fdiv_contract_reassoc_cosh_sinh_reassoc_multiple_uses_sinh(double %a) {
; CHECK-LABEL: @fdiv_contract_reassoc_cosh_sinh_reassoc_multiple_uses_sinh(
; CHECK-NEXT: [[TMP1:%.*]] = call reassoc double @cosh(double [[A:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = call reassoc double @sinh(double [[A]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc contract double [[TMP1]], [[TMP2]]
; CHECK-NEXT: call void @use(double [[TMP2]])
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv contract reassoc double %cosh, %sinh
call void @use(double %sinh)
ret double %div
}

define double @fdiv_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_contract_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_contract_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc contract double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc contract double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv contract reassoc double %cosh, %sinh
ret double %div
}

define fp128 @fdiv_coshl_sinhl_reassoc(fp128 %a){
%cosh = call reassoc fp128 @coshl(fp128 %a)
%sinh = call reassoc fp128 @sinhl(fp128 %a)
%div = fdiv reassoc fp128 %cosh, %sinh
ret fp128 %div
}

define float @fdiv_coshf_sinhf_reassoc(float %a){
; CHECK-LABEL: @fdiv_coshf_sinhf_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc float @tanhf(float [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc float 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret float [[DIV]]
;
%coshf = call reassoc float @coshf(float %a)
%sinhf = call reassoc float @sinhf(float %a)
%div = fdiv reassoc float %coshf, %sinhf
ret float %div
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the tests with contract combinations?

Copy link
Contributor

@arsenm arsenm May 8, 2024

Choose a reason for hiding this comment

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

Test preservation of other flags? Maybe !fpmath too, just in case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tests for all the flags I could find (if I missed any please let me know) that test for flag preservation in case of replacement. I also added the contract combinations and the target triples.

define double @fdiv_fast_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_fast_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc ninf double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc ninf double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv ninf reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_nnan_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_nnan_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc nnan double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc nnan double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv nnan reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_nsz_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_nsz_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc nsz double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc nsz double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv nsz reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_arcp_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_arcp_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc arcp double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc arcp double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv arcp reassoc double %cosh, %sinh
ret double %div
}

define double @fdiv_afn_reassoc_cosh_sinh_reassoc(double %a){
; CHECK-LABEL: @fdiv_afn_reassoc_cosh_sinh_reassoc(
; CHECK-NEXT: [[TANH:%.*]] = call reassoc afn double @tanh(double [[A:%.*]])
; CHECK-NEXT: [[DIV:%.*]] = fdiv reassoc afn double 1.000000e+00, [[TANH]]
; CHECK-NEXT: ret double [[DIV]]
;
%cosh = call reassoc double @cosh(double %a)
%sinh = call reassoc double @sinh(double %a)
%div = fdiv afn reassoc double %cosh, %sinh
ret double %div
}

declare double @cosh(double)
declare float @coshf(float)
declare fp128 @coshl(fp128)

declare double @sinh(double)
declare float @sinhf(float)
declare fp128 @sinhl(fp128)
Comment on lines +217 to +219
Copy link
Contributor

Choose a reason for hiding this comment

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

Is TargetLibraryInfo still oblivious to half functions?


declare void @use(double)
Loading