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

refactor overdub/context code #41

Merged
merged 11 commits into from
May 10, 2018
Merged

refactor overdub/context code #41

merged 11 commits into from
May 10, 2018

Conversation

jrevels
Copy link
Collaborator

@jrevels jrevels commented Apr 20, 2018

  • keep caller with varargs while passing through overdub operations
  • remove Overdub object, which only existed previously in order to work around unwanted type normalization
  • the object formally known as TraceConfig is now the context struct itself, generated along with a context-specific tag type at context definition time
  • change overdub macro to pass in context object to make box construction/destruction easier

@vtjnash This (along with JuliaLang/julia#26826) renders the test case from #31 inferrable! (cc @maleadt)

julia> @code_warntype Cassette.overdub_execute(Ctx(), unsafe_load, pointer(Float32[]))
Variables:
  ctx<optimized out>
  args::Tuple{typeof(unsafe_load),Ptr{Float32}}
  i<optimized out>

Body:
  begin
      Core.SSAValue(15) = (Core.getfield)(args::Tuple{typeof(unsafe_load),Ptr{Float32}}, 2)::Ptr{Float32}
      Core.SSAValue(74) = (pointerref)(Core.SSAValue(15), 1, 1)::Float32
      return Core.SSAValue(74)
  end::Float32

The intermediate IntrinsicFunction call doesn't "look" inferred, though. Why is that? It looks similar to code_warntype output we saw when testing the original vararg_type_container trick, which IIRC, was just due to the respecialization happening at a later stage. Is this just the same thing? Is it a problem at all? EDIT: We fixed the problem Jameson highlighted below, so I've updated the code snippet above (even the intermediate stuff is now inferred).

- keep caller with varargs while passing through overdub operations
- remove Overdub object, which only existed previously in order to work around unwanted type normalization
- the object formally known as TraceConfig is now the context struct itself, generated along with a context-specific tag type at context definition time
- change overdub macro to pass in context object to make box construction/destruction easier
@jrevels
Copy link
Collaborator Author

jrevels commented Apr 20, 2018

@Keno may also be happy to know that the above inference works fine even when aggressive_constant_propagation == false

@vtjnash
Copy link

vtjnash commented Apr 22, 2018

Why is that?

Most likely that means you’ve gotten the cache lookup algorithm wrong (failing to find the match)

…tor of that part of Cassette

The Box API will likely be mostly the same, but the implementation could be very, very different; it doesn't make sense to do a bunch of extra work to keep those tests passing right now, since the impending changes won't be incremental (and Cassette isn't released anyway, so nobody should be depending on this right now).

Of course, the old implementation will always be in the history for reference
@jrevels
Copy link
Collaborator Author

jrevels commented Apr 30, 2018

Note that this (along with JuliaLang/julia#26826) fixes #5

@jrevels
Copy link
Collaborator Author

jrevels commented May 10, 2018

The current thing that's keeping this branch from being useable:

julia> using Cassette: @context, @prehook, @overdub

julia> @context Ctx

julia> @prehook (f::Any)(args...) where {__CONTEXT__<:Ctx} = println(f, args)

julia> function foo(T)
           if isa(T, UnionAll)
               return UnionAll(T.var, foo(T.body))
           elseif isa(T, Union)
               return Union{foo(T.a), foo(T.b)}
           else
               return T
           end
       end
foo (generic function with 1 method)

julia> S = Tuple{Vararg{T}} where T
Tuple{Vararg{T,N} where N} where T

julia> foo(S)
Tuple{Vararg{T,N} where N} where T

julia> @overdub(Ctx(), foo(S))
foo(Tuple{Vararg{T,N} where N} where T,)
isa(Tuple{Vararg{T,N} where N} where T, UnionAll)
Base.getproperty(Tuple{Vararg{T,N} where N} where T, :var)
getfield(Tuple{Vararg{T,N} where N} where T, :var)
Base.getproperty(Tuple{Vararg{T,N} where N} where T, :body)
getfield(Tuple{Vararg{T,N} where N} where T, :body)
foo(Tuple{Vararg{T,N} where N},)
isa(Tuple{Vararg{T,N} where N}, UnionAll)
Base.getproperty(Tuple{Vararg{T,N} where N}, :var)
getfield(Tuple{Vararg{T,N} where N}, :var)
ERROR: type DataType has no field var
Stacktrace:
 [1] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:9 [inlined]
 [2] overdub_recurse at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155 [inlined]
 [3] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [4] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba0,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0xa96e611db2fca271}}, ::typeof(foo), ::Type{Tuple{Vararg{T,N} where N} where T}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:3
 [5] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [6] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba0,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0xa96e611db2fca271}}, ::typeof(foo), ::Type{Tuple{Vararg{T,N} where N} where T}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:3
 [7] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [8] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba0,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0xa96e611db2fca271}}, ::getfield(Main, Symbol("##4#5")), ::Ctx{Cassette.Unused,0x0000000000006ba0,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0xa96e611db2fca271}}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155
 [9] top-level scope at /Users/jarrettrevels/.julia/v0.7/Cassette/src/macros.jl:68

EDIT: more minimal test function (same context as above):

julia> bar(T) = isa(T, UnionAll) ? bar(T.body) : T
bar (generic function with 1 method)

julia> @overdub(Ctx(), bar(Vector{T} where T))
TypeVar(:T,)
Core.apply_type(Union,)
Core.apply_type(Array{T,1} where T, T)
UnionAll(T, Array{T,1})
bar(Array{T,1} where T,)
isa(Array{T,1} where T, UnionAll)
Base.getproperty(Array{T,1} where T, :body)
getfield(Array{T,1} where T, :body)
bar(Array{T,1},)
isa(Array{T,1}, UnionAll)
Array{T,1}

julia> bar(Vector{T} where T)
Array{T,1}

julia> @overdub(Ctx(), bar(Vector{Vector{T}} where T))
TypeVar(:T,)
Core.apply_type(Union,)
Core.apply_type(Array{T,1} where T, T)
Core.apply_type(Array{T,1} where T, Array{T,1})
UnionAll(T, Array{Array{T,1},1})
bar(Array{Array{T,1},1} where T,)
isa(Array{Array{T,1},1} where T, UnionAll)
Base.getproperty(Array{Array{T,1},1} where T, :body)
getfield(Array{Array{T,1},1} where T, :body)
bar(Array{Array{T,1},1},)
isa(Array{Array{T,1},1}, UnionAll)
Base.getproperty(Array{Array{T,1},1}, :body)
getfield(Array{Array{T,1},1}, :body)
ERROR: type DataType has no field body
Stacktrace:
 [1] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:9 [inlined]
 [2] overdub_recurse at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155 [inlined]
 [3] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [4] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba7,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0x5e60b7525025d2c1}}, ::typeof(bar), ::Type{Array{Array{T,1},1} where T}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155
 [5] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [6] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba7,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0x5e60b7525025d2c1}}, ::typeof(bar), ::Type{Array{Array{T,1},1} where T}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155
 [7] overdub_execute at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:34 [inlined]
 [8] overdub_recurse(::Ctx{Cassette.Unused,0x0000000000006ba7,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0x5e60b7525025d2c1}}, ::getfield(Main, Symbol("##16#17")), ::Ctx{Cassette.Unused,0x0000000000006ba7,Cassette.Unused,getfield(Main, Symbol("##CtxTag#1559")){Nothing,0x5e60b7525025d2c1}}) at /Users/jarrettrevels/.julia/v0.7/Cassette/src/overdub.jl:155
 [9] top-level scope at /Users/jarrettrevels/.julia/v0.7/Cassette/src/macros.jl:68

julia> bar(Vector{Vector{T}} where T)
Array{Array{T,1},1}

@jrevels
Copy link
Collaborator Author

jrevels commented May 10, 2018

Note that the above errors occur even before JuliaLang/julia#26826; I ran it on a build of JuliaLang/julia@598ebff and it was still broken.

Furthermore, it's broken on Cassette master, as well (which I hadn't realized).

That being the case, I'll merge this and file a new issue to track the problem.

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 this pull request may close these issues.

2 participants