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

inference: remove MAX_TYPE_DEPTH limit, rewrite tmerge #26628

Merged
merged 5 commits into from
Apr 6, 2018
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Mar 27, 2018

These arbitrary limits don't seem necessary any more (at least for make test), and weren't being used where they would have been needed the most (Tuple) anyways. This also tries to improve the associativity of the algorithm and slowing down the convergence, with an eye towards giving better results when Nothing gets joined into a mix of abstract types:

Core.Compiler.tmerge(Union{Complex{Int32}, Complex{Int64}, Nothing}, Complex{Int16})
`Any` => `Union{Nothing, Complex}`

fix #26610
close #23077

`type_too_complex` / `MAX_TYPE_DEPTH` is not well-defined
(this was the last remaining usage)

`_typename(a::Vararg)` does not mean what the author probably thought / intended
(it is the same as ..., and thus was dead code)

This limit is now handled with more surgical precision by `limit_type_size` during recursion,
so we can be more permissive during iteration here.
and slow down convergence rate of Tuple and UnionAll types
@vtjnash
Copy link
Member Author

vtjnash commented Mar 30, 2018

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

1 similar comment
@quinnj
Copy link
Member

quinnj commented Apr 1, 2018

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

@nanosoldier
Copy link
Collaborator

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

@vtjnash
Copy link
Member Author

vtjnash commented Apr 4, 2018

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

@nanosoldier
Copy link
Collaborator

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

@vtjnash vtjnash force-pushed the jn/typelimits branch 2 times, most recently from 593af8f to c3ebc9e Compare April 5, 2018 05:57
… Int}`

While the new optimizer should handle this better,
the optimal efficient way to handle this is to make the missingness entirely disjoin:
`Union{Nothing, Tuple{Something, Int}}`

And make each `val` variable SSA, for better inference
@vtjnash
Copy link
Member Author

vtjnash commented Apr 5, 2018

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

@nanosoldier
Copy link
Collaborator

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

@vtjnash
Copy link
Member Author

vtjnash commented Apr 5, 2018

great. will merge soon if no objections

@vtjnash vtjnash merged commit b89e012 into master Apr 6, 2018
@vtjnash vtjnash deleted the jn/typelimits branch April 6, 2018 16:21
omus added a commit to JuliaTime/TimeZones.jl that referenced this pull request Apr 14, 2018
omus added a commit to JuliaTime/TimeZones.jl that referenced this pull request Apr 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Union types turn into Any when splatted
3 participants