-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
inline invoke #9642
inline invoke #9642
Conversation
Wow, this is impressive. Very good work. I'll review it in more detail later. A related change that should help a lot, and is likely easier to implement than this, is to add a case to |
@JeffBezanson Yeah, I did it this way only because this is slightly easier to do for me... I tried to replace it with anonymous function like this after trying to inline it but somehow it segfault at compile time (of sysimg). if meth.tvars == () && !meth.isstage
return Expr(:call, meth.func, argexprs...)
end I know this is totaly a hack but as I was testing with |
I guess I'm still not very familiar with the order of these passes it still cannot inline invoke if it is wrapped.... yuyichao% cat invoke-inline.jl
#!/usr/bin/julia -f
invoke_wrap(f::Function, ts::Tuple, args...) = invoke(f, ts, args...)
f(a, b) = a + b
g_invoke() = invoke(f, (Any, Any), 1, 2)
g_invoke_wrap() = invoke_wrap(f, (Any, Any), 1, 2)
println(@code_typed g_invoke())
println(@code_typed g_invoke_wrap())
yyc2:~/projects/explore/julia/invoke
yuyichao% ./invoke-inline.jl
Any[:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,Int64,18],Any[:_var1,Int64,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 7:
_var0 = 1
_var1 = 2
return (top(box))(Int64,(top(add_int))(_var0::Int64,_var1::Int64))
end::Int64))))]
Any[:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,(Int64,Int64),0],Any[:_var1,Function,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 8:
_var1 = f::ANY
return invoke(_var1::F,(Any,Any),1,2)::ANY
end::ANY))))] |
atypes_l = Type[atypes...] | ||
err_label = genlabel(sv) | ||
after_err_label = genlabel(sv) | ||
for i in 1:length(atypes_l) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when inlining arguments, you need to go in reverse order to preserve the order-of-execution (see https://github.com/yuyichao/julia/blob/inline-invoke/base/inference.jl#L2770-L2771 for an example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is what I'm trying to do here. I've seen that part but I didn't really understand why. Isn't the arguments evaluated in the order they appears in the code?
Also the code generated seems to be currect here and that's why I didn't bother too much before sending the PR
julia> @noinline function get_next()
global counter
counter = counter + 1
return counter
end
get_next (generic function with 1 method)
julia> f(a, b) = (a, b)
f (generic function with 1 method)
julia> g() = invoke(f, (Integer, Integer), get_next(), get_next())
g (generic function with 1 method)
julia> @code_typed g()
1-element Array{Any,1}:
:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,Any,18],Any[:_var1,Any,18]],Any[]], :(begin # none, line 1:
_var0 = get_next()::Any
_var1 = get_next()::Any
unless (isa)(_var0,Integer)::Bool goto 1
unless (isa)(_var1,Integer)::Bool goto 1
goto 2
1:
(error)("invoke: argument type error")::Union()
2:
return (top(tuple))(_var0::Integer,_var1::Integer)::(Integer,Integer)
end::(Integer,Integer)))))
julia> g()
ERROR: counter not defined
in get_next at ./no file:3
in g at ./none:1
julia> counter = 0
0
julia> g()
(1,2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm waiting for a build now, but what if you change the second argument to invoke to (Any, Integer)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, i wasn't paying attention to the fact that you are always copying the argument to a temporary variable. there's no need to do that if the argument is effect_free
/ affect_free
(the difference linguistically is subtle: effect-free means that it does not cause an effect on surrounding code (e.g. pure), whereas affect-free means it is not affected by surrounding code (e.g. immutable))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type check is done after all arguments are evaluated so changing to (Any, Integer)
does not affect the evaluation of the arguments at all. (which is the same schematics with calling invoke
function)
(Actually I think I might miss the case where evaluating the second argument to invoke
has side effect)....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, forgot to paste the output................
julia> @noinline function get_next()
global counter
counter = counter + 1
return counter
end
get_next (generic function with 1 method)
julia> counter = 0
0
julia> f(a, b) = (a, b)
f (generic function with 1 method)
julia> g() = invoke(f, (Any, Integer), get_next(), get_next())
g (generic function with 1 method)
julia> @code_typed g()
1-element Array{Any,1}:
g() :($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,Any,18],Any[:_var1,Any,18]],Any[]], :(begin # none, line 1:
_var0 = get_next()::Any
_var1 = get_next()::Any
unless (isa)(_var1,Integer)::Bool goto 1
goto 2
1:
(error)("invoke: argument type error")::Union()
2:
return (top(tuple))(_var0::Any,_var1::Integer)::(Any,Integer)
end::(Any,Integer)))))
julia> g()
(1,2)
Actually it seems that the inlining of function is not very smart in some cases either (when the function is assigned to a variable) even if the variable is a const yuyichao% cat invoke-inline.jl
#!/usr/bin/julia -f
invoke_wrap(f::Function, ts::Tuple, args...) = invoke(f, ts, args...)
f(a, b) = a + b
g_invoke() = invoke(f, (Any, Any), 1, 2)
g_invoke_wrap() = invoke_wrap(f, (Any, Any), 1, 2)
function g_invoke2()
const tmp_f = f
invoke(tmp_f, (Any, Any), 1, 2)
end
function g_call2()
const tmp_f = f
tmp_f(1, 2)
end
println(@code_typed g_invoke())
println(@code_typed g_invoke_wrap())
println(@code_typed g_invoke2())
println(@code_typed g_call2())
yyc2:~/projects/explore/julia/invoke
yuyichao% ./invoke-inline.jl
Any[:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,Int64,18],Any[:_var1,Int64,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 7:
_var0 = 1
_var1 = 2
return (top(box))(Int64,(top(add_int))(_var0::Int64,_var1::Int64))
end::Int64))))]
Any[:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,(Int64,Int64),0],Any[:_var1,Function,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 8:
_var1 = f::ANY
return invoke(_var1::F,(Any,Any),1,2)::ANY
end::ANY))))]
Any[:($(Expr(:lambda, Any[], Any[Any[:tmp_f],Any[Any[:tmp_f,Function,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 10:
const tmp_f::ANY
tmp_f = f::ANY # line 11:
return invoke(tmp_f::F,(Any,Any),1,2)::ANY
end::ANY))))]
Any[:($(Expr(:lambda, Any[], Any[Any[:tmp_f],Any[Any[:tmp_f,Function,18]],Any[]], :(begin # /home/yuyichao/projects/explore/julia/invoke/invoke-inline.jl, line 14:
const tmp_f::ANY
tmp_f = f::ANY # line 15:
return (tmp_f::F)(1,2)::ANY
end::ANY))))] |
aae9fa1
to
33d8fe7
Compare
@JeffBezanson Hopefully the last commit addresses some of your suggestions. There are some copy-paste's and I've also changed julia> @noinline f(a, b) = a + b
f (generic function with 1 method)
julia> g() = invoke(f, (Any, Any), 1, 2)
g (generic function with 1 method)
julia> @code_typed g()
1-element Array{Any,1}:
:($(Expr(:lambda, Any[], Any[Any[:_var0,:_var1],Any[Any[:_var0,Int64,18],Any[:_var1,Int64,18]],Any[]], :(begin # none, line 1:
return invoke(f,(Any,Any),1,2)::Int64
end::Int64))))
julia> @code_llvm g();
define i64 @julia_g_42944() {
top:
%0 = call i64 @julia_f_42945(i64 1, i64 2), !dbg !8
ret i64 %0, !dbg !8
} |
9a020f2
to
9e60f7b
Compare
Another issue is that the lookup is done several times during compilation (3 times for a non-inlineable invoke). Not sure what is the best way to solve this... |
P.S. any objection if I make a |
i thought most would be regardless, I wouldn't object.
I think method lookup does this too. perhaps not the best, but perhaps unavoidable |
I was thinking about just combinine the pop and return. which is slightly more difficult to combine into a macro (since C doesn't using a prefix in this case is probably good enough. It won't be worse than
Would be possible in C++, although in that case there's much better way to
Yes it is. And probably not less than invoke, which is why I didn't bother
|
a9bbe0a
to
18ed87a
Compare
cc @vtjnash our CI is failing assertions on mac and segfaulting on Windows. Any ideas how to fix? |
18ed87a
to
265d755
Compare
@tkelman It might have sth to do with this PR but AFAIK it happens after I did a rebase on the current master..... Also is there a reason that |
We've seen those on master also. Best strategy is probably to replace that assert with jlbacktrace and dump a little more info about the environment |
I see.. I checked the master CI just now and thought it was fine..... Now I saw this on the other PR I have... which really shouldn't have anything to do with it..... |
If I had to guess, it's probably because codegen assumes that if it comes across a type-tuple, it should be unboxed. but that isn't true of the current tuple implementation |
d0a6cce
to
b250712
Compare
Just noticed another issue. Since the inference of julia> f(a, b) = a + b
f (generic function with 1 method)
julia> g(args...) = invoke(f, (Any, Any), args...)
g (generic function with 1 method)
julia> k() = g(1, 2)
k (generic function with 1 method)
julia> @code_typed k()
1-element Array{Any,1}:
:($(Expr(:lambda, Any[], Any[Any[:_var2,:_var3,:_var4],Any[Any[:_var2,(Int64,Int64),0],Any[:_var3,Int64,18],Any[:_var4,Int64,18]],Any[]], :(begin # none, line 1:
(Any,Any)
_var3 = 1
_var4 = 2
return (top(box))(Int64,(top(add_int))(_var3::Int64,_var4::Int64))
end::Any)))) |
@yuyichao will your pull-request also improve the following?
Output:
It will obviously help |
I don't think it can improve any of these. The invoke inliner is not smarter than the generic function inliner and I haven't change anything wrt the normal function inliner. It would make it easier to improve test3 if test1 is improved though ..... |
6d8cf16
to
422c541
Compare
…ays evaluate for now before I figure out how to use effect_free() properly
422c541
to
1c8816b
Compare
Close this one in favor of #10964 (and possibly other ones) since this pr needs major refactor to keep up with the current master. |
This is my attempt to improve issue #9608. I'm pretty sure there is sth I'm still missing though....
Benchmark with the following script
Before,
After