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

Possible regression in Julia 1.11 for throw block #643

Closed
kbarros opened this issue Jun 30, 2024 · 3 comments
Closed

Possible regression in Julia 1.11 for throw block #643

kbarros opened this issue Jun 30, 2024 · 3 comments

Comments

@kbarros
Copy link

kbarros commented Jun 30, 2024

The following code passes in 1.10, but flags an error in 1.11.0-rc1

using JET

function g(x)
    throw("Should JET ignore this? $x")
end

@test_opt unoptimize_throw_blocks=true g(1/3)

The error is:

[ Info: tracking Base
  ═════ 4 possible errors found ═════
  ┌ g(x::Float64) @ Main ./REPL[2]:2
  │┌ string(::String, ::Float64) @ Base ./strings/io.jl:189
  ││┌ print_to_string(::String, ::Float64) @ Base ./strings/io.jl:150
  │││┌ _unsafe_take!(io::IOBuffer) @ Base ./iobuffer.jl:504
  ││││┌ wrap(::Type{Array}, m::MemoryRef{UInt8}, l::Int64) @ Base ./array.jl:3099
  │││││ failed to optimize due to recursion: Base.wrap(::Type{Array}, ::MemoryRef{UInt8}, ::Int64)
  ││││└────────────────────
  │││┌ print_to_string(::String, ::Vararg{Any}) @ Base ./strings/io.jl:143
  ││││ runtime dispatch detected: Base._str_sizehint(%17::Any)::Int64
  │││└────────────────────
  │││┌ print_to_string(::String, ::Vararg{Any}) @ Base ./strings/io.jl:148
  ││││ runtime dispatch detected: print(%59::IOBuffer, %97::Any)::Any
  │││└────────────────────
  │││┌ string(::String, ::Int64, ::String, ::Tuple{Int64}, ::String, ::Int64, ::String, ::Int64, ::String) @ Base ./strings/io.jl:189
  ││││ failed to optimize due to recursion: string(::String, ::Int64, ::String, ::Tuple{Int64}, ::String, ::Int64, ::String, ::Int64, ::String)
  │││└────────────────────

My understanding of skip_unoptimized_throw_blocks = true (the default) is that it should ignore unoptimization in g, which is known to throw. Is that correct?

@aviatesk
Copy link
Owner

aviatesk commented Jul 1, 2024

Yes, cases like this should be hidden by skip_unoptimized_throw_blocks=true, but due to the current implementation issue of report_opt, such cases can sometimes be reported. I will consider re-implementing report_opt.
However, regarding this particular issue, it is expected to be fixed in v1.11 rc2 and the release version without waiting for a fix on the JET side (xref: JuliaLang/julia#54942).

aviatesk added a commit that referenced this issue Jul 3, 2024
In the analysis performed by `OptAnalyzer`, when the
`skip_unoptimized_throw_blocks` configuration is enabled, dynamic
dispatch on `throw` code path is supposed to be ignored. However,
cached reports of non-inlined callees on the `throw` code path were not
ignored, leading to confusion (#643). This commit addresses the issue by
overloading the propagation of reports from callees in abstract
interpretation.

While the issue has been resolved, the resulting code is quite hacky,
and improvements to these interfaces are certainly desirable.
aviatesk added a commit that referenced this issue Jul 3, 2024
In the analysis performed by `OptAnalyzer`, when the
`skip_unoptimized_throw_blocks` configuration is enabled, dynamic
dispatch on `throw` code path is supposed to be ignored. However,
cached reports of non-inlined callees on the `throw` code path were not
ignored, leading to confusion (#643). This commit addresses the issue by
overloading the propagation of reports from callees in abstract
interpretation.

While the issue has been resolved, the resulting code is quite hacky,
and improvements to these interfaces are certainly desirable.
@kbarros
Copy link
Author

kbarros commented Jul 3, 2024

Thanks! Is it possible to also make JET detect error as well as throw? In my actual use case, the exception originates with an error:

using JET

function g(x)
    error("Please ignore this: $x")
end

@test_opt unoptimize_throw_blocks=true g(1/3)

@kbarros
Copy link
Author

kbarros commented Jul 10, 2024

I created a follow-up issue #647 to track the remaining issue. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants