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

Jump to bottom

update to JuliaLang/julia#44389 #303

Merged
merged 1 commit into from
Mar 3, 2022
Merged

update to JuliaLang/julia#44389 #303

merged 1 commit into from
Mar 3, 2022

Conversation

aviatesk
Copy link
Contributor

@aviatesk aviatesk commented Mar 3, 2022

JuliaLang/julia#44389 removed method_table(::AbstractInterpreter, ::InferenceState) interface,
and now we should overload method_table(::AbstractInterpreter) instead.

@codecov
Copy link

codecov bot commented Mar 3, 2022 •
edited
Loading

Codecov Report

Merging #303 (bddc0c1) into master (55ae222) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #303      +/-   ##
==========================================
+ Coverage   85.53%   85.55%   +0.01%     
==========================================
  Files          22       22              
  Lines        2102     2271     +169     
==========================================
+ Hits         1798     1943     +145     
- Misses        304      328      +24
Impacted Files Coverage Δ
src/jlgen.jl 79.65% <0.00%> (-2.17%) ⬇️
src/error.jl 28.00% <0.00%> (-2.44%) ⬇️
src/ptx.jl 92.82% <0.00%> (-2.42%) ⬇️
src/spirv.jl 88.30% <0.00%> (-1.14%) ⬇️
src/irgen.jl 93.20% <0.00%> (-0.96%) ⬇️
src/gcn.jl 68.80% <0.00%> (-0.13%) ⬇️
src/execution.jl 100.00% <0.00%> (ø)
src/mcgen.jl 97.36% <0.00%> (+0.07%) ⬆️
src/validation.jl 96.99% <0.00%> (+0.11%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55ae222...bddc0c1. Read the comment docs.

maleadt
maleadt approved these changes Mar 3, 2022
View reviewed changes
Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

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

LGTM otherwise!

src/jlgen.jl Outdated Show resolved Hide resolved
@aviatesk aviatesk force-pushed the avi/update branch 2 times, most recently from d8df39f to 776cbd5 Compare March 3, 2022 09:18
@aviatesk
update to JuliaLang/julia#44389
bddc0c1 
JuliaLang/julia#44389 removed `method_table(::AbstractInterpreter, ::InferenceState)` interface,
and now we should overload `method_table(::AbstractInterpreter)` instead.
@maleadt
Copy link
Member

maleadt commented Mar 3, 2022

Failure to install SPIRV translator should be resolved once JuliaPackaging/Yggdrasil#4291 lands and is tagged.

@aviatesk aviatesk closed this Mar 3, 2022
@aviatesk aviatesk reopened this Mar 3, 2022
@maleadt maleadt merged commit fb6d180 into JuliaGPU:master Mar 3, 2022
@aviatesk aviatesk deleted the avi/update branch March 3, 2022 11:52
@maleadt
Copy link
Member

maleadt commented Mar 3, 2022

I'm pretty sure this caused the following:

  Got exception outside of a @test
  MethodError: no method matching findsup(::Type{Tuple{typeof(evalpoly), Any, Tuple}}, ::Core.Compiler.OverlayMethodTable)
  Closest candidates are:
    findsup(::Type, ::Core.Compiler.InternalMethodTable) at compiler/methodtable.jl:94
  Stacktrace:
    [1] abstract_invoke(interp::GPUCompiler.GPUInterpreter, ::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1477
    [2] abstract_call_known(interp::GPUCompiler.GPUInterpreter, f::Any, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1523
    [3] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1691
    [4] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1670
    [5] abstract_eval_statement(interp::GPUCompiler.GPUInterpreter, e::Any, vtypes::Vector{Core.Compiler.VarState}, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1812
    [6] typeinf_local(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2276
    [7] typeinf_nocycle(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2372
    [8] _typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:230
    [9] typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:213
   [10] typeinf_edge(interp::GPUCompiler.GPUInterpreter, method::Method, atype::Any, sparams::Core.SimpleVector, caller::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:866
   [11] abstract_call_method(interp::GPUCompiler.GPUInterpreter, method::Method, sig::Any, sparams::Core.SimpleVector, hardlimit::Bool, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:597
   [12] abstract_call_gf_by_type(interp::GPUCompiler.GPUInterpreter, f::Any, arginfo::Core.Compiler.ArgInfo, atype::Any, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:130
   [13] abstract_call_known(interp::GPUCompiler.GPUInterpreter, f::Any, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1633
  --- the last 11 lines are repeated 2 more times ---
   [36] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1691
   [37] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1670
   [38] abstract_eval_statement(interp::GPUCompiler.GPUInterpreter, e::Any, vtypes::Vector{Core.Compiler.VarState}, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1812
   [39] typeinf_local(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2250
   [40] typeinf_nocycle(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2372
   [41] _typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:230
   [42] typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:213
   [43] typeinf_edge(interp::GPUCompiler.GPUInterpreter, method::Method, atype::Any, sparams::Core.SimpleVector, caller::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:866
   [44] abstract_call_method(interp::GPUCompiler.GPUInterpreter, method::Method, sig::Any, sparams::Core.SimpleVector, hardlimit::Bool, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:597
   [45] abstract_call_gf_by_type(interp::GPUCompiler.GPUInterpreter, f::Any, arginfo::Core.Compiler.ArgInfo, atype::Any, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:130
   [46] abstract_call_known(interp::GPUCompiler.GPUInterpreter, f::Any, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1633
   [47] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState, max_methods::Int64)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1691
   [48] abstract_call(interp::GPUCompiler.GPUInterpreter, arginfo::Core.Compiler.ArgInfo, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1670
   [49] abstract_eval_statement(interp::GPUCompiler.GPUInterpreter, e::Any, vtypes::Vector{Core.Compiler.VarState}, sv::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:1812
   [50] typeinf_local(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2276
  --- the last 11 lines are repeated 1 more time ---
   [62] typeinf_nocycle(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/abstractinterpretation.jl:2372
   [63] _typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:230
   [64] typeinf(interp::GPUCompiler.GPUInterpreter, frame::Core.Compiler.InferenceState)
      @ Core.Compiler ./compiler/typeinfer.jl:213
   [65] typeinf_ext(interp::GPUCompiler.GPUInterpreter, mi::Core.MethodInstance)
      @ Core.Compiler ./compiler/typeinfer.jl:947
   [66] typeinf_ext_toplevel(interp::GPUCompiler.GPUInterpreter, linfo::Core.MethodInstance)
      @ Core.Compiler ./compiler/typeinfer.jl:980

I guess we were still using the internal method table in some places, as we didn't implement one of the method_table methods. Any thoughts? Can we just fall back to the contained method table like CachedMethodTable used to do? https://github.com/JuliaLang/julia/pull/44240/files#diff-b75febecd7ab93bee6f56364837ee90d74ed0f9409d337adeecf9e96dab4b9d8L125 Or do we need overlay-specific semantics?

@aviatesk
Copy link
Contributor Author

aviatesk commented Mar 3, 2022

findsup needs to be implemented for OverlayMethodTable. Previously abstract_invoke didn't use method_table interface correctly and didn't use OverlayMethodTable.

@aviatesk aviatesk mentioned this pull request Mar 4, 2022
Merged
@aviatesk
Copy link
Contributor Author

aviatesk commented Mar 4, 2022

See JuliaLang/julia#44448.

@maleadt
Copy link
Member

maleadt commented Mar 4, 2022

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maleadt maleadt maleadt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aviatesk @maleadt