-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
improve constant prop in gcd #41258
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
improve constant prop in gcd #41258
Conversation
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
|
Just posting here for posterity: The underlying issue here is #41694. A recursive implementation also doesn't seem to help: julia> function _gcd(u, v)
u == v && return u, v
if u > v
u, v = v, u
end
v -= u
v >>= trailing_zeros(v)
return _gcd(u, v)
end
_gcd (generic function with 1 method)
julia> function mygcd(a::T, b::T) where {T<:Base.BitInteger}
za = trailing_zeros(a)
zb = trailing_zeros(b)
k = min(za, zb)
u = unsigned(abs(a >> za))
v = unsigned(abs(b >> zb))
u, v = _gcd(u, v)
r = u << k
# T(r) would throw InexactError; we want OverflowError instead
r > typemax(T) && Base.__throw_gcd_overflow(a, b)
r % T
end
mygcd (generic function with 1 method)
julia> code_typed() do
mygcd(2, 3)
end
1-element Vector{Any}:
CodeInfo(
1 ── %1 = (0x0000000000000001 === 0x0000000000000001)::Bool
└─── goto #3 if not %1
2 ── %3 = Core.tuple(0x0000000000000001, 0x0000000000000001)::Tuple{UInt64, UInt64}
└─── goto #6
3 ── %5 = Base.ult_int(0x0000000000000001, 0x0000000000000001)::Bool
└─── goto #5 if not %5
4 ── nothing::Nothing
5 ┄─ %8 = φ (#4 => 0x0000000000000001, #3 => 0x0000000000000001)::UInt64
│ %9 = φ (#4 => 0x0000000000000001, #3 => 0x0000000000000001)::UInt64
│ %10 = Base.sub_int(%9, %8)::UInt64
│ %11 = Base.cttz_int(%10)::UInt64
│ %12 = Base.bitcast(Int64, %11)::Int64
│ %13 = Base.sle_int(0, %12)::Bool
│ %14 = Base.bitcast(UInt64, %12)::UInt64
│ %15 = Base.lshr_int(%10, %14)::UInt64
│ %16 = Base.neg_int(%12)::Int64
│ %17 = Base.bitcast(UInt64, %16)::UInt64
│ %18 = Base.shl_int(%10, %17)::UInt64
│ %19 = Base.ifelse(%13, %15, %18)::UInt64
│ %20 = invoke Main._gcd(%8::UInt64, %19::UInt64)::Tuple{UInt64, UInt64}
└─── goto #6
6 ┄─ %22 = φ (#2 => %3, #5 => %20)::Tuple{UInt64, UInt64}
└─── goto #7
7 ── %24 = Base.getfield(%22, 1)::UInt64
│ %25 = Base.shl_int(%24, 0x0000000000000000)::UInt64
│ %26 = Base.lshr_int(%24, 0x0000000000000000)::UInt64
│ %27 = Base.ifelse(true, %25, %26)::UInt64
│ %28 = Base.ult_int(0x7fffffffffffffff, %27)::Bool
│ %29 = Base.or_int(false, %28)::Bool
└─── goto #9 if not %29
8 ── %31 = Base.__throw_gcd_overflow::typeof(Base.__throw_gcd_overflow)
│ invoke %31(2::Int64, 3::Int64)::Union{}
└─── unreachable
9 ── %34 = Base.bitcast(Int64, %27)::Int64
└─── goto #10
10 ─ return %34
) => Int64Until that is fixed, I hope this is a reasonable workaround, since rational constructors are probably one of the places where constant prop is quite crucial. |
vchuravy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash your changes :)
With this PR:
It is a bit unfortunate to have to resort to
@purehere, but I couldnot get it to constant fold any other way. I don't think this usage
should be problematic since the method only accepts
BitIntegers andonly ever calls methods that really shouldn't be redefined.
fixes #32024