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.

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 effects for Float64^Int on 32 bit #54934

Merged

Conversation

oscardssmith
Copy link
Member

fixes #54910 properly.

The recursion heuristic was getting mad at this for some reason...

base/math.jl Outdated Show resolved Hide resolved
@oscardssmith oscardssmith added system:32-bit Affects only 32-bit systems compiler:effects effect analysis labels Jun 26, 2024
@oscardssmith oscardssmith merged commit 9fecc19 into JuliaLang:master Jun 26, 2024
9 checks passed
@oscardssmith oscardssmith deleted the os/fix-Float64-Int-effects branch June 26, 2024 04:38
@aviatesk
Copy link
Sponsor Member

I'm curious as to why recursion heuristics become an issue, so if you have time, could you check whether clamp(n::Int32, Int64) is type stable on a 32-bit machine?

@oscardssmith
Copy link
Member Author

Not sure what's up...

julia> @code_warntype clamp(1, Int64)
MethodInstance for clamp(::Int32, ::Type{Int64})
  from clamp(x, ::Type{T}) where T<:Integer @ Base.Math math.jl:122
Static Parameters
  T = Int64
Arguments
  #self#::Core.Const(clamp)
  x::Int32
  _::Core.Const(Int64)
Locals
  hi::Int64
  lo::Int64
Body::Int64
1 ─ %1  = Base.Math.typemin::Core.Const(typemin)
│   %2  = $(Expr(:static_parameter, 1))::Core.Const(Int64)
│         (lo = (%1)(%2))
│   %4  = Base.Math.typemax::Core.Const(typemax)
│   %5  = $(Expr(:static_parameter, 1))::Core.Const(Int64)
│         (hi = (%4)(%5))
│   %7  = Base.Math.:>::Core.Const(>)
│   %8  = hi::Core.Const(9223372036854775807)
│   %9  = (%7)(x, %8)::Bool
└──       goto #3 if not %9
2 ─ %11 = hi::Core.Const(9223372036854775807)
└──       return %11
3 ─ %13 = Base.Math.:<::Core.Const(<)
│   %14 = lo::Core.Const(-9223372036854775808)
│   %15 = (%13)(x, %14)::Bool
└──       goto #5 if not %15
4 ─ %17 = lo::Core.Const(-9223372036854775808)
└──       return %17
5 ─ %19 = Base.Math.convert::Core.Const(convert)
│   %20 = $(Expr(:static_parameter, 1))::Core.Const(Int64)
│   %21 = (%19)(%20, x)::Int64
└──       return %21

@aviatesk
Copy link
Sponsor Member

aviatesk commented Jun 26, 2024

Strange. It's scary. If this is type stable, then recursion heuristics shouldn't be triggered. However, it might be possible that when clamp(::Int32, Type{Int64}) is inferred for the first time, if there are some functions that are not yet defined, causing the inferred return type to be Any and cached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:effects effect analysis system:32-bit Affects only 32-bit systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants