Inline invoke (take 3) plus a few other tweaks #18444

yuyichao · 2016-09-11T11:52:36Z

Replaces #10964 (take 2). Expr(:invoke) makes this significantly easier to implement =)

Summary of changes,

Deprecate the tuple form of invoke and use the tuple type form instead.

It's much easier to infer/optimize.
Inline invoke call.

~~Based on #18438 since I hit it when working on this PR.~~ (merged)

Close #9608

yuyichao · 2016-09-11T12:14:38Z

Travis OSX failed with a network issue Restarted.

vtjnash · 2016-09-11T19:59:32Z

base/inference.jl

@@ -1297,7 +1297,7 @@ function ⊑(a::ANY, b::ANY)
    end
 end

-widenconst(c::Const) = typeof(c.val)
+widenconst(c::Const) = type_typeof(c.val)


typeof should always be correct

But widens too much?

Widens what? Const.val isn't a Type

K. I think I was probably confused by the printing and thought it could be.

julia> Core.Inference.print(Core.Inference.Const(sin)) Core.Inference.Const(val=Base.#sin())

tkelman · 2016-09-12T03:41:54Z

base/inference.jl

+                    local all = true
+                    local stmts = []
+                    local aei = ex.args[i]
+                    for ty in (ti::Union).types; local ty


local ty on a separate line?

Sure. Just note that this is simply moved from where it was.

vtjnash · 2016-09-16T04:36:28Z

src/gf.c

+    jl_method_t *method = entry->func.method;
+    jl_typemap_entry_t *tm = NULL;
+    if (method->invokes.unknown != NULL) {
+        tm = jl_typemap_assoc_by_type(method->invokes, tt, NULL, 0, 1,


inexact possibly needs to be true (1)

vtjnash · 2016-09-16T04:38:08Z

src/gf.c

+    if (func->invokes.unknown == NULL)
+        func->invokes.unknown = jl_nothing;
+
+    jl_method_instance_t *mfunc = cache_method(mt, &func->invokes, entry->func.value,


we may not want to insert into the invokes cache, unless tt was a leaf signature

vtjnash · 2016-09-16T04:40:37Z

src/gf.c

+                                              jl_tupletype_t *tt)
+{
+    if (!jl_is_leaf_type((jl_value_t*)tt))
+        return jl_nothing;


overly strict, but seems this should work. I guess you're mimicking jl_get_specialization1? You also need to check that none of the elements are types of types like that other method too then.

overly strict

Yeah. There are currently very feel users and most of them should have leaf type (forwarding) so I didn't bother handling more complex cases.

I guess you're mimicking jl_get_specialization1

Right.

You also need to check that none of the elements are types of types like that other method too then.

Added. I assume the comments below (inexact and caching) does not apply anymore then?

kshyatt · 2016-09-21T03:48:40Z

Rebase?

yuyichao · 2016-10-09T23:41:12Z

The TODO is finished and this is ready for review.

tkelman · 2016-10-11T11:01:35Z

@nanosoldier runbenchmarks(ALL, vs=":master")

yuyichao · 2016-10-15T13:03:11Z

get_invoke_func

Do you mean to make it return an actual function? In which case we'll need to create a new type in it and it doesn't sound easy or efficient. Or do you mean to use sth like Expr(:invoke) to call it? Which sounds like a good api but will make it harder to share code with the normal function call.

I'm also not sure how much easier this would be. The main complexity in this actually comes from type check and argument order. The type check is a mostly independent patch and commit. The argument order won't be an issue at all when we have linear IR.

What about leaving the type check part out (the last commit) and think about changing invoke implementation later? I feel like having a get_invoke_func won't help too much right now but can still be useful since it might be able to share code path with #13984

JeffBezanson · 2017-01-08T19:04:10Z

@yuyichao @vtjnash Should we try to get this into 0.6? I might want to use invoke to fix #7357.

yuyichao · 2017-01-08T22:38:05Z

Is #18444 (comment) (last paragraph) ok for now?

JeffBezanson · 2017-01-08T22:52:46Z

Yes, I like the idea of using all but the last commit here for now. That commit has a lot of extra complexity, and IIUC we get all our usual optimizations without it?

yuyichao · 2017-01-08T22:57:06Z

Yes, the last one should only affect some cases when certain arguments to the invoke is not inferred.

…reason about

Fix #9608

yuyichao · 2017-01-09T01:18:00Z

Rebased.

yuyichao · 2017-01-09T04:10:01Z

Just in case

@nanosoldier runbenchmarks(ALL, vs=":master")

nanosoldier · 2017-01-09T08:21:21Z

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

tkelman · 2017-01-09T10:25:01Z

The deprecation of the tuple form is worth mentioning in NEWS

…liaLang#18444).

…8444). (#21817)

…8444). (#21817) (cherry picked from commit d052e88)

yuyichao mentioned this pull request Sep 11, 2016

Inline callsite of invoke when possible (invoke improvement No. 1) #10964

Closed

yuyichao added performance Must go faster compiler:inference Type inference labels Sep 11, 2016

yuyichao force-pushed the yyc/invoke/inline branch from eb7ef56 to 28922e0 Compare September 11, 2016 12:32

yuyichao force-pushed the yyc/invoke/inline branch from 28922e0 to cdafa58 Compare September 11, 2016 15:54

vtjnash reviewed Sep 11, 2016
View reviewed changes

yuyichao force-pushed the yyc/invoke/inline branch from cdafa58 to a41a8f6 Compare September 12, 2016 01:31

tkelman reviewed Sep 12, 2016
View reviewed changes

yuyichao mentioned this pull request Sep 13, 2016

Better definition of broadcast(::Function, ::Integer) #18468

Merged

yuyichao force-pushed the yyc/invoke/inline branch from a41a8f6 to 3e21bc8 Compare September 14, 2016 15:25

vtjnash reviewed Sep 16, 2016

View reviewed changes

yuyichao mentioned this pull request Sep 16, 2016

Avoid infinite method error on bootstrap error #18438

Merged

yuyichao force-pushed the yyc/invoke/inline branch from 3e21bc8 to 05068b1 Compare September 25, 2016 15:11

yuyichao force-pushed the yyc/invoke/inline branch 2 times, most recently from 689235a to 45345bd Compare October 9, 2016 23:29

yuyichao changed the title ~~WIP: Inline invoke (take 3) plus a few other tweaks~~ Inline invoke (take 3) plus a few other tweaks Oct 9, 2016

yuyichao force-pushed the yyc/invoke/inline branch from 45345bd to e03b717 Compare January 9, 2017 01:01

yuyichao added 6 commits January 9, 2017 09:05

Replace all use of invoke to use Tuple arguments

bdd244e

Deprecate invoke with tuple argument

46df749

Make invoke_NF a top level function so that it is slightly easier to …

153e514

…reason about

Make invoke_NF more compatible with invoke

a430c07

Inline invoke (take 3)

4d7a054

Fix #9608

More invoke tests

b1c47ec

yuyichao force-pushed the yyc/invoke/inline branch from e03b717 to b1c47ec Compare January 9, 2017 01:05

yuyichao mentioned this pull request Jan 9, 2017

Fix use of invoke fhs/ZipFile.jl#32

Merged

JeffBezanson approved these changes Jan 9, 2017

View reviewed changes

yuyichao mentioned this pull request Jan 9, 2017

Fix use of invoke FixedEffects/FixedEffectModels.jl#10

Merged

yuyichao mentioned this pull request Jan 9, 2017

Performance of invoke #9608

Closed

yuyichao merged commit f65673d into master Jan 9, 2017

yuyichao deleted the yyc/invoke/inline branch January 9, 2017 08:59

tkelman added the needs news A NEWS entry is required for this change label Jan 9, 2017

tkelman mentioned this pull request May 11, 2017

Make sure NEWS.md lists all 0.6 deprecations and breaking changes #21475

Closed

Sacha0 added a commit to Sacha0/julia that referenced this pull request May 12, 2017

Add NEWS.md entry for deprecation of invoke's tuple-of-types form (Ju…

b16058d

…liaLang#18444).

Sacha0 mentioned this pull request May 12, 2017

add NEWS.md entry for deprecation of invoke's tuple-of-types form #21817

Merged

tkelman pushed a commit that referenced this pull request May 12, 2017

Add NEWS.md entry for deprecation of invoke's tuple-of-types form (#1…

d052e88

…8444). (#21817)

tkelman pushed a commit that referenced this pull request May 14, 2017

Add NEWS.md entry for deprecation of invoke's tuple-of-types form (#1…

2a92944

…8444). (#21817) (cherry picked from commit d052e88)

KristofferC removed the needs news A NEWS entry is required for this change label Nov 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inline invoke (take 3) plus a few other tweaks #18444

Inline invoke (take 3) plus a few other tweaks #18444

yuyichao commented Sep 11, 2016 •

edited

Loading

yuyichao commented Sep 11, 2016

vtjnash Sep 11, 2016

yuyichao Sep 11, 2016

vtjnash Sep 11, 2016

yuyichao Sep 11, 2016

tkelman Sep 12, 2016

yuyichao Sep 13, 2016

vtjnash Sep 16, 2016

vtjnash Sep 16, 2016

vtjnash Sep 16, 2016 •

edited

Loading

yuyichao Sep 25, 2016

kshyatt commented Sep 21, 2016

yuyichao commented Oct 9, 2016

tkelman commented Oct 11, 2016

yuyichao commented Oct 15, 2016

JeffBezanson commented Jan 8, 2017

yuyichao commented Jan 8, 2017

JeffBezanson commented Jan 8, 2017

yuyichao commented Jan 8, 2017

yuyichao commented Jan 9, 2017

yuyichao commented Jan 9, 2017

nanosoldier commented Jan 9, 2017

tkelman commented Jan 9, 2017

Inline invoke (take 3) plus a few other tweaks #18444

Inline invoke (take 3) plus a few other tweaks #18444

Conversation

yuyichao commented Sep 11, 2016 • edited Loading

yuyichao commented Sep 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vtjnash Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshyatt commented Sep 21, 2016

yuyichao commented Oct 9, 2016

tkelman commented Oct 11, 2016

yuyichao commented Oct 15, 2016

JeffBezanson commented Jan 8, 2017

yuyichao commented Jan 8, 2017

JeffBezanson commented Jan 8, 2017

yuyichao commented Jan 8, 2017

yuyichao commented Jan 9, 2017

yuyichao commented Jan 9, 2017

nanosoldier commented Jan 9, 2017

tkelman commented Jan 9, 2017

yuyichao commented Sep 11, 2016 •

edited

Loading

vtjnash Sep 16, 2016 •

edited

Loading