Skip to content

[ConstantFold] Fold erf and erff when the input parameter is a constant value. #113079

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 22, 2024
Merged

[ConstantFold] Fold erf and erff when the input parameter is a constant value. #113079

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d6360d2
const fold erf
c8ef Oct 20, 2024
f7ece18
target triple
c8ef Oct 20, 2024
c29a50f
fix test
c8ef Oct 20, 2024
270d720
add strict test
c8ef Oct 20, 2024
aaf3681
revert
c8ef Oct 21, 2024
50dd390
Merge branch 'main' into math
c8ef Oct 22, 2024
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
14 changes: 11 additions & 3 deletions llvm/lib/Analysis/ConstantFolding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1671,8 +1671,8 @@ bool llvm::canConstantFoldCallTo(const CallBase *Call, const Function *F) {
Name == "cos" || Name == "cosf" ||
Name == "cosh" || Name == "coshf";
case 'e':
return Name == "exp" || Name == "expf" ||
Name == "exp2" || Name == "exp2f";
return Name == "exp" || Name == "expf" || Name == "exp2" ||
Name == "exp2f" || Name == "erf" || Name == "erff";
case 'f':
return Name == "fabs" || Name == "fabsf" ||
Name == "floor" || Name == "floorf" ||
Expand Down Expand Up @@ -2411,6 +2411,11 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
break;
case LibFunc_logl:
return nullptr;
case LibFunc_erf:
case LibFunc_erff:
if (TLI->has(Func))
return ConstantFoldFP(erf, APF, Ty);
break;
case LibFunc_nearbyint:
case LibFunc_nearbyintf:
case LibFunc_rint:
Expand Down Expand Up @@ -3596,7 +3601,6 @@ bool llvm::isMathLibCallNoop(const CallBase *Call,
// Per POSIX, this MAY fail if Op is denormal. We choose not failing.
return true;


case LibFunc_asinl:
case LibFunc_asin:
case LibFunc_asinf:
Expand Down Expand Up @@ -3624,6 +3628,10 @@ bool llvm::isMathLibCallNoop(const CallBase *Call,
case LibFunc_sqrtf:
return Op.isNaN() || Op.isZero() || !Op.isNegative();

case LibFunc_erf:
case LibFunc_erff:
return true;
Copy link
Contributor

@arsenm arsenm Oct 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm unclear on if and when errno is written. The linux man page says no. The C standard seems to suggest it raises a range error, which depending on platform configuration may be errno and/or fp exception

Copy link
Contributor Author

@c8ef c8ef Oct 21, 2024

Choose a reason for hiding this comment

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

Maybe we can revert this change and keep the original call as it is?

Copy link
Contributor Author

@c8ef c8ef Oct 21, 2024

Choose a reason for hiding this comment

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

The current implementation is safe because underflow will be caught by FE_UNDERFLOW and not folded at all. Hopefully it is good now?

Copy link
Contributor

@arsenm arsenm Oct 22, 2024

Choose a reason for hiding this comment

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

I don't think this is actually that safe, llvm isn't compiling with strictfp enabled

Copy link
Contributor Author

@c8ef c8ef Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When I locally test passing a parameter less than DBL_MIN * (std::sqrt(M_PI) / 2), I get a floating-point exception on both Windows and Linux.

Copy link
Contributor

@arsenm arsenm Oct 22, 2024

Choose a reason for hiding this comment

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

Yes, it will probably work on most hosts in practice but it's not structurally correct

Copy link
Contributor Author

@c8ef c8ef Oct 22, 2024

Choose a reason for hiding this comment

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

Would it be better to explicitly test if the absolute value of APF is greater than DBL_MIN * (std::sqrt(M_PI) / 2)?

Copy link
Contributor Author

@c8ef c8ef Oct 22, 2024

Choose a reason for hiding this comment

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

It appears that the handling of current overflow and underflow in isMathLibCallNoop is similar to the exp function. Therefore, after removing the code snippet in isMathLibCallNoop, it will simply return false, indicating that it is not a no-op.

Copy link
Contributor

@arsenm arsenm Oct 29, 2024

Choose a reason for hiding this comment

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

You would have to enable strictfp support (which barely works in llvm), or avoid calling the host function in any of the edge cases that you are relying on the exception

Copy link
Contributor Author

@c8ef c8ef Oct 29, 2024

Choose a reason for hiding this comment

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

Actually, I previously tried using erf, but I still couldn't achieve folding because it returns false in isMathLibCallNoop. Therefore, it's at least safe. Perhaps we should calculate the underflow boundary and add it into isMathLibCallNoop to enable constant folding.


// FIXME: Add more functions: sqrt_finite, atanh, expm1, log1p,
// maybe others?
default:
Expand Down
215 changes: 215 additions & 0 deletions llvm/test/Transforms/InstCombine/erf.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -passes=instcombine -mtriple x86_64-unknown-linux-gnu -S | FileCheck %s

define float @erff_const() {
; CHECK-LABEL: define float @erff_const() {
; CHECK-NEXT: ret float 0x3FE0A7EF60000000
;
%r = call float @erff(float 5.000000e-01)
ret float %r
}

define double @erf_const() {
; CHECK-LABEL: define double @erf_const() {
; CHECK-NEXT: ret double 0xBFE0A7EF5C18EDD2
;
%r = call double @erf(double -5.000000e-01)
ret double %r
}

define float @erff_zero() {
; CHECK-LABEL: define float @erff_zero() {
; CHECK-NEXT: ret float 0.000000e+00
;
%r = call float @erff(float 0.000000e+00)
ret float %r
}

define double @erf_zero() {
; CHECK-LABEL: define double @erf_zero() {
; CHECK-NEXT: ret double 0.000000e+00
;
%r = call double @erf(double 0.000000e+00)
Copy link
Contributor

@arsenm arsenm Oct 20, 2024

Choose a reason for hiding this comment

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

None of these cases folded?

Copy link
Contributor Author

@c8ef c8ef Oct 20, 2024

Choose a reason for hiding this comment

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

IIUC the only cases that are not folded are NaN and Inf; zero input will be folded as expected.

Copy link
Contributor

@arsenm arsenm Oct 20, 2024

Choose a reason for hiding this comment

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

The call wasn't deleted though.

Plus the linux docs suggest it doesn't ever set errno

Copy link
Contributor Author

@c8ef c8ef Oct 20, 2024

Choose a reason for hiding this comment

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

The call wasn't deleted though.

Other libcall test case exhibits similar behavior, but it does not seem to affect the final fold result. I believe the unused instruction will be deleted anyway?

Plus the linux docs suggest it doesn't ever set errno

Yes, it is still blocked by the previously discussed issue and will be addressed later.

llvm-project/llvm/lib/Analysis/ConstantFolding.cpp

Lines 2237 to 2241 in d6360d2

/// We only fold functions with finite arguments. Folding NaN and inf is
/// likely to be aborted with an exception anyway, and some host libms
/// have known errors raising exceptions.
if (!U.isFinite())
return nullptr;

Copy link
Contributor

@arsenm arsenm Oct 20, 2024

Choose a reason for hiding this comment

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

Other libcall test case exhibits similar behavior, but it does not seem to affect the final fold result. I believe the unused instruction will be deleted anyway?

Then those are also broken. This call needs to be immediately deleted

Copy link
Contributor Author

@c8ef c8ef Oct 20, 2024

Choose a reason for hiding this comment

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

Not sure why the calls are not being deleted. The problem might be at the callsite, specifically in instsimplify.

Copy link
Contributor

@arsenm arsenm Oct 20, 2024

Choose a reason for hiding this comment

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

Hm, it does seem these are deleted later. I would still expect them to be immediately killed, but this seems to be a more widespread issue

Copy link
Contributor Author

@c8ef c8ef Oct 20, 2024

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

@c8ef c8ef Oct 20, 2024

Choose a reason for hiding this comment

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

There are two issues here:

  1. The function should be marked as willreturn to ensure it is deleted from the worklist. This will be handled by InferFunctionAttrs during compilation, but is not currently observed in testing.
  2. isMathLibCallNoop should include the relevant library call.

ret double %r
}

define float @erff_neg_zero() {
; CHECK-LABEL: define float @erff_neg_zero() {
; CHECK-NEXT: ret float -0.000000e+00
;
%r = call float @erff(float -0.000000e+00)
ret float %r
}

define double @erf_neg_zero() {
; CHECK-LABEL: define double @erf_neg_zero() {
; CHECK-NEXT: ret double -0.000000e+00
;
%r = call double @erf(double -0.000000e+00)
ret double %r
}

define float @erff_inf() {
; CHECK-LABEL: define float @erff_inf() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0x7FF0000000000000)
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0x7FF0000000000000)
ret float %r
}

define double @erf_inf() {
; CHECK-LABEL: define double @erf_inf() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0x7FF0000000000000)
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0x7FF0000000000000)
ret double %r
}

define float @erff_inf_memory_none() {
; CHECK-LABEL: define float @erff_inf_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0x7FF0000000000000) #[[ATTR1:[0-9]+]]
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0x7FF0000000000000) readnone
ret float %r
}

define double @erf_inf_memory_none() {
; CHECK-LABEL: define double @erf_inf_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0x7FF0000000000000) #[[ATTR1]]
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0x7FF0000000000000) readnone
ret double %r
}

define float @erff_neg_inf() {
; CHECK-LABEL: define float @erff_neg_inf() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0xFFF0000000000000)
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0xFFF0000000000000)
ret float %r
}

define double @erf_neg_inf() {
; CHECK-LABEL: define double @erf_neg_inf() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0xFFF0000000000000)
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0xFFF0000000000000)
ret double %r
}

define float @erff_neg_inf_memory_none() {
; CHECK-LABEL: define float @erff_neg_inf_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0xFFF0000000000000) #[[ATTR1]]
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0xFFF0000000000000) readnone
ret float %r
}

define double @erf_neg_inf_memory_none() {
; CHECK-LABEL: define double @erf_neg_inf_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0xFFF0000000000000) #[[ATTR1]]
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0xFFF0000000000000) readnone
ret double %r
}

define float @erff_nan() {
; CHECK-LABEL: define float @erff_nan() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0x7FF8000000000000)
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0x7FF8000000000000)
ret float %r
}

define double @erf_nan() {
; CHECK-LABEL: define double @erf_nan() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0x7FF8000000000000)
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0x7FF8000000000000)
ret double %r
}

define float @erff_nan_memory_none() {
; CHECK-LABEL: define float @erff_nan_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0x7FF8000000000000) #[[ATTR1]]
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0x7FF8000000000000) readnone
ret float %r
}

define double @erf_nan_memory_none() {
; CHECK-LABEL: define double @erf_nan_memory_none() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0x7FF8000000000000) #[[ATTR1]]
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0x7FF8000000000000) readnone
ret double %r
}

define float @erff_poison() {
; CHECK-LABEL: define float @erff_poison() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float poison)
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float poison)
ret float %r
}

define double @erf_poison() {
; CHECK-LABEL: define double @erf_poison() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double poison)
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double poison)
ret double %r
}

define float @erff_const_strictfp() {
; CHECK-LABEL: define float @erff_const_strictfp() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 5.000000e-01) #[[ATTR2:[0-9]+]]
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 5.000000e-01) strictfp
ret float %r
}

define double @erf_const_strictfp() {
; CHECK-LABEL: define double @erf_const_strictfp() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double -5.000000e-01) #[[ATTR2]]
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double -5.000000e-01) strictfp
ret double %r
}

define float @erff_nan_strictfp() {
; CHECK-LABEL: define float @erff_nan_strictfp() {
; CHECK-NEXT: [[R:%.*]] = call float @erff(float 0x7FF8000000000000) #[[ATTR2]]
; CHECK-NEXT: ret float [[R]]
;
%r = call float @erff(float 0x7FF8000000000000) strictfp
ret float %r
}

define double @erf_nan_strictfp() {
; CHECK-LABEL: define double @erf_nan_strictfp() {
; CHECK-NEXT: [[R:%.*]] = call double @erf(double 0x7FF8000000000000) #[[ATTR2]]
; CHECK-NEXT: ret double [[R]]
;
%r = call double @erf(double 0x7FF8000000000000) strictfp
ret double %r
}

declare float @erff(float) willreturn
declare double @erf(double) willreturn
Loading