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

expected return statement, got JuliaInterpreter.SSAValue(2) #288

Closed
fredrikekre opened this issue May 23, 2019 · 11 comments · Fixed by #289
Closed

expected return statement, got JuliaInterpreter.SSAValue(2) #288

fredrikekre opened this issue May 23, 2019 · 11 comments · Fixed by #289

Comments

@fredrikekre
Copy link
Contributor

With the following (minified) package

module ReviseBug

using Tensors

struct A
    x::Tensor{2,3,Float64,9}
end

function foo()
    x = 1.0 * one(Tensor{2,3,Float64,9}) # removing the multiplication makes things work
    return A(x)
end

const a = foo()

end # module

I get the following when trying to revise the module:

julia> using Revise, ReviseBug

shell> touch src/ReviseBug.jl

julia> 1+1
┌ Warning: omitting call expression (ReviseBug.foo)() in ("none", 0)
└ @ Revise ~/.julia/packages/Revise/UtBAC/src/lowered.jl:169
┌ Error: evaluation error
│   mod = ReviseBug
│   ex = :(const a = foo())
│   exception =
│    expected return statement, got a = JuliaInterpreter.SSAValue(2)
│    Stacktrace:
│     [1] error(::String, ::Expr) at ./error.jl:42
│     [2] get_return(::JuliaInterpreter.Frame) at /home/fredrik/.julia/packages/JuliaInterpreter/6LimI/src/interpret.jl:598
│     [3] #methods_by_execution!#14(::Bool, ::Bool, ::Function, ::Any, ::Revise.CodeTrackingMethodInfo, ::Dict{Module,Array{Expr,1}}, ::JuliaInterpreter.Frame) at /home/fredrik/.julia/packages/Revise/UtBAC/src/lowered.jl:185
│     [4] #methods_by_execution! at ./none:0 [inlined]
│     [5] #methods_by_execution!#9(::Base.Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:define,),Tuple{Bool}}}, ::Function, ::Any, ::Revise.CodeTrackingMethodInfo, ::Dict{Module,Array{Expr,1}}, ::Module, ::Expr) at /home/fredrik/.julia/packages/Revise/UtBAC/src/lowered.jl:49
│     [6] #methods_by_execution! at ./none:0 [inlined]
│     [7] #eval_with_signatures#59(::Bool, ::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::Module, ::Expr) at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:344
│     [8] #eval_with_signatures at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:0 [inlined]
│     [9] #instantiate_sigs!#60(::Bool, ::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::OrderedCollections.OrderedDict{Module,OrderedCollections.OrderedDict{Revise.RelocatableExpr,Union{Nothing, Array{Any,1}}}}) at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:352
│     [10] instantiate_sigs! at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:349 [inlined]
│     [11] maybe_parse_from_cache!(::Revise.PkgData, ::String) at /home/fredrik/.julia/packages/Revise/UtBAC/src/pkgs.jl:224
│     [12] handle_deletions at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:473 [inlined]
│     [13] revise() at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:526
│     [14] revise at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:551 [inlined]
│     [15] run_backend(::REPL.REPLBackend) at /home/fredrik/.julia/packages/Revise/UtBAC/src/Revise.jl:941
│     [16] (::getfield(Revise, Symbol("##74#76")){REPL.REPLBackend})() at ./task.jl:259
└ @ Revise ~/.julia/packages/Revise/UtBAC/src/lowered.jl:52
2

Maybe there is something funky going on with @pure-usage in Tensors.jl? As noted, removing the multiplication solves the problem.

@fredrikekre
Copy link
Contributor Author

$ jlpkg st -m Revise
Project ReviseBug v0.1.0
    Status `~/dev/ReviseBug/Manifest.toml`
  [da1fd8a2] CodeTracking v0.5.7
  [aa1ae85d] JuliaInterpreter v0.5.1
  [6f1432cf] LoweredCodeUtils v0.3.4
  [bac558e1] OrderedCollections v1.1.0
  [295af30f] Revise v2.1.4
  [8ba89e20] Distributed 
  [7b1f6079] FileWatching 
  [76f85450] LibGit2 
  [44cfe95a] Pkg 
  [3fa0cd96] REPL 
  [cf7118a7] UUIDs 
  [4ec0a83e] Unicode

@timholy
Copy link
Member

timholy commented May 23, 2019

What Julia version? If recent, please try master branch of JuliaInterpreter, may have been fixed by #286

@fredrikekre
Copy link
Contributor Author

fredrikekre commented May 23, 2019

1.1.1, but also happens on julia master (JuliaLang/julia@efd794e), also happens with JuliaInterpreter master (04d1fb5).

@timholy
Copy link
Member

timholy commented May 23, 2019

Problematic call is here, it's a this intrinsic must be compiled to be called error. We're failing to detect an llvmcall emitted by a @generated function. Help getting a Revise-independent (pure @interpret) MWE would be appreciated.

@fredrikekre
Copy link
Contributor Author

Ah okay. Yea, I had problems in the early days of the new JuliaInterpreter with SIMD.jl, but forgot about it. Here is an example, still with Tensors but I can probably work out a pure julia example later tonight:

julia> using JuliaInterpreter, Tensors

julia> f() = 1.0 * one(Tensor{2,3})
f (generic function with 1 method)

julia> @interpret f()
ERROR: this intrinsic must be compiled to be called

@timholy
Copy link
Member

timholy commented May 23, 2019

That's probably good enough, we can make Tensors a test dependency. Thanks!

@KristofferC
Copy link
Member

So, dup of #112?

@fredrikekre
Copy link
Contributor Author

I just got this error again with [email protected] when running the original code.

@timholy
Copy link
Member

timholy commented Jul 25, 2019

Drat. Forgot about this until today. I poked at it, and it's a world-age issue (it should work if you edit it a second time). The issue is that our tests run at top-level and hence the world age can update, whereas with Revise it's run from inside a function.

I only see one option: when we define compiled handlers for llvmcall (and probably ccall), we may need to do something evil: extract the method tables and set the world age to that of the current running task. Thoughts, @KristofferC? (I know how to do that and will implement it if appropriate, I'm just wonder if it's something we can countenance.)

@KristofferC KristofferC reopened this Jul 25, 2019
@KristofferC
Copy link
Member

I don't fully grok methods_by_execution! but is there no equivalent of invokelatest we could use here?

If you feel messing with world ages is the best we can do then let's go with that.

@timholy
Copy link
Member

timholy commented Jul 25, 2019

Good question. I was worried we'd have to add it lots of places, but you prompted me to check again. Turns out we can fix it by making this line invokelatest. I think that's a pretty rare path, and the benchmarks don't seem different, so seems like perhaps worth trying?

timholy added a commit that referenced this issue Jul 25, 2019
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

Successfully merging a pull request may close this issue.

3 participants