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.

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

fix literal-pow to return the right type when the base is -1 #53736

Closed
wants to merge 3 commits into from
Closed

fix literal-pow to return the right type when the base is -1 #53736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34f8126
fix literal-pow to return the right type when the base is -1
oscardssmith Mar 14, 2024
a5f8b2a
add tests
oscardssmith Mar 14, 2024
db3677b
Update base/intfuncs.jl
oscardssmith Mar 15, 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
19 changes: 16 additions & 3 deletions base/intfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,19 @@ const HWNumber = Union{HWReal, Complex{<:HWReal}, Rational{<:HWReal}}
@inline literal_pow(::typeof(^), x::HWNumber, ::Val{1}) = x
@inline literal_pow(::typeof(^), x::HWNumber, ::Val{2}) = x*x
@inline literal_pow(::typeof(^), x::HWNumber, ::Val{3}) = x*x*x
@inline literal_pow(::typeof(^), x::HWNumber, ::Val{-1}) = inv(x)
@inline literal_pow(::typeof(^), x::HWNumber, ::Val{-2}) = (i=inv(x); i*i)
@inline function literal_pow(::typeof(^), x::HWNumber, ::Val{-1})
if x == -one(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

This change introduces even more type instability, now depending on the value of the x argument.

Other question: why special-case -1 and not 1?

Copy link
Member Author

Choose a reason for hiding this comment

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

the reason to specialize only on -1 is that inv(x) often has a different type than x, but x*x generally is ok. what is a fade where this version is type unstable? unitful?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant depending on the first argument: typeof ((-1)^-1) == Int, while typeof(1^-1) == Float64)

return x
end
inv(x)
end
@inline function literal_pow(::typeof(^), x::HWNumber, ::Val{-2})
if x == -one(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment on line 373

return isodd(p) ? one(x) : -one(x)
end
i=inv(x)
return i*i
end

# don't use the inv(x) transformation here since float^p is slightly more accurate
@inline literal_pow(::typeof(^), x::AbstractFloat, ::Val{p}) where {p} = x^p
Expand All @@ -379,7 +390,9 @@ const HWNumber = Union{HWReal, Complex{<:HWReal}, Rational{<:HWReal}}
# for other types, define x^-n as inv(x)^n so that negative literal powers can
# be computed in a type-stable way even for e.g. integers.
@inline function literal_pow(f::typeof(^), x, ::Val{p}) where {p}
if p < 0
if x == -one(x)
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment on line 373

return isodd(p) ? one(x) : -one(x)
elseif p < 0
if x isa BitInteger64
f(Float64(x), p) # inv would cause rounding, while Float64^Integer is able to compensate the inverse
else
Expand Down
7 changes: 7 additions & 0 deletions test/math.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1459,6 +1459,13 @@ end
# two cases where we have observed > 1 ULP in the past
@test 0.0013653274095082324^-97.60372292227069 == 4.088393948750035e279
@test 8.758520413376658e-5^70.55863059215994 == 5.052076767078296e-287
# test literal pow for base = -1
@test (-1)^(-1) === (-1)^((-1,)[1])
@test (-1)^(-2) === (-1)^((-2,)[1])
@test (-1)^(-3) === (-1)^((-3,)[1])
@test (-1)^(1) === (-1)^((1,)[1])
@test (-1)^(2) === (-1)^((2,)[1])
@test (-1)^(3) === (-1)^((3,)[1])
end

# Test that sqrt behaves correctly and doesn't exhibit fp80 double rounding.
Expand Down