-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
constant-folding rational constants #32024
Comments
Is there any hope for this? The generated code for |
I experimented with the new |
Does |
Not as far as I can tell. (In |
I believe the problem is the construction of the julia> 1//typemin(Int)
ERROR: ArgumentError: invalid rational: denominator can't be typemin(Int64)
Stacktrace:
[1] __throw_rational_argerror_typemin(T::Type)
@ Base ./rational.jl:20
[2] checked_den
@ ./rational.jl:24 [inlined]
[3] Rational{Int64}(num::Int64, den::Int64)
@ Base ./rational.jl:35
[4] Rational
@ ./rational.jl:38 [inlined]
[5] //(n::Int64, d::Int64)
@ Base ./rational.jl:61
[6] top-level scope
@ REPL[43]:1 The problem in the original post could be simplified to this: julia> h() = 2//3
h (generic function with 2 methods)
julia> @code_typed h()
CodeInfo(
1 ─ %1 = invoke Base.divgcd(2::Int64, 3::Int64)::Tuple{Int64, Int64}
│ %2 = Base.getfield(%1, 1)::Int64
│ %3 = Base.getfield(%1, 2)::Int64
│ %4 = Base.slt_int(%3, 0)::Bool
└── goto #5 if not %4
2 ─ %6 = Base.neg_int(%3)::Int64
│ %7 = Base.slt_int(%6, 0)::Bool
└── goto #4 if not %7
3 ─ invoke Base.__throw_rational_argerror_typemin(Int64::Type)::Union{}
└── unreachable
4 ─ %11 = Base.neg_int(%2)::Int64
5 ┄ %12 = φ (#4 => %11, #1 => %2)::Int64
│ %13 = φ (#4 => %6, #1 => %3)::Int64
│ %14 = %new(Rational{Int64}, %12, %13)::Rational{Int64}
└── goto #6
6 ─ goto #7
7 ─ goto #8
8 ─ goto #9
9 ─ return %14
) => Rational{Int64}
julia> @btime h()
15.165 ns (0 allocations: 0 bytes)
2//3 |
Here is one sloppy solution: just bypass the constructor and don't check for invalid rationals: julia> (//)(x::T, y::T) where {T <: Integer} = Base.unsafe_rational(T, x, y)
// (generic function with 1 method)
julia> f(x) = x * (2//3)
f (generic function with 1 method)
julia> @code_llvm f(1.1)
; @ REPL[3]:1 within `f'
define double @julia_f_291(double %0) {
top:
; ┌ @ promotion.jl:322 within `*' @ float.jl:332
%1 = fmul double %0, 0x3FE5555555555555
; └
ret double %1
} |
Hm, why would that block constant propagation? |
A quick workaround for this is to use f(x) = x * Base.unsafe_rational(2, 3) instead. (Edit: Sorry, overread Mason's answer right above) |
With this PR: ```julia julia> f(x) = x * (2//3) f (generic function with 1 method) julia> @code_typed f(2.3) CodeInfo( 1 ─ %1 = Base.mul_float(x, 0.6666666666666666)::Float64 └── return %1 ) => Float64 ``` It is a bit unfortunate to have to resort to `@pure` here, but I could not get it to constant fold any other way. I don't think this usage should be problematic since the method only accepts `BitInteger`s and only ever calls methods that really shouldn't be redefined. fixes #32024
With this PR: ```julia julia> f(x) = x * (2//3) f (generic function with 1 method) julia> @code_typed f(2.3) CodeInfo( 1 ─ %1 = Base.mul_float(x, 0.6666666666666666)::Float64 └── return %1 ) => Float64 ``` It is a bit unfortunate to have to resort to `@pure` here, but I could not get it to constant fold any other way. I don't think this usage should be problematic since the method only accepts `BitInteger`s and only ever calls methods that really shouldn't be redefined. fixes #32024
With this PR: ```julia julia> f(x) = x * (2//3) f (generic function with 1 method) julia> @code_typed f(2.3) CodeInfo( 1 ─ %1 = Base.mul_float(x, 0.6666666666666666)::Float64 └── return %1 ) => Float64 ``` It is a bit unfortunate to have to resort to `@pure` here, but I could not get it to constant fold any other way. I don't think this usage should be problematic since the method only accepts `BitInteger`s and only ever calls methods that really shouldn't be redefined. fixes #32024
It is nice to use rationals for constants like
2//3
that are supposed to be precision-independent, in a context likef(x) = x * (2//3)
, but they still impose a performance cost because converting them toFloat32
orFloat64
involves a runtime division. Can this be made to happen at compile-time?apparently doesn't work —
@code_native f(2.3)
still shows a runtime division.This feature was requested in #14324, which was "fixed" in #24362 by @vtjnash — does that mean that there is supposed to be an annotation that enables constant-folding here? (If not, why was #14324 closed?)
In contrast,
g(x) = x * (oftype(x,2) / oftype(x,3))
works (@code_native g(3.7)
shows that the constant 2/3 ≈ 0.6666666666666666 is constructed at compile time) but is obviously more cumbersome to write.The text was updated successfully, but these errors were encountered: