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

kill isleaftype #25496

Merged
merged 5 commits into from
Jan 19, 2018
Merged

kill isleaftype #25496

merged 5 commits into from
Jan 19, 2018

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Jan 10, 2018

As discussed in previous issues (listed below), this finishes the deprecation of isleaftype and adds/corrects a number of other type-property queries and their usages (esp. including isbits). This correction also includes a number of changes that makes inference more powerful (by leveraging the removal of isleaftype to return more correct answers, yay!).

Most of the new computations are cached inside a datatype upon construction:

// properties of `parameters`
     uint8_t hasfreetypevars; // majority part of isconcrete computation
+    uint8_t isconcretetype; // where this type can have instances
+    uint8_t isdispatchtuple; // aka isleaftupletype
// properties of `types`
+    uint8_t isbitstype; // relevant query for C-api and type-parameters
+    uint8_t zeroinit; // if one or more fields requires zero-initialization
+    uint8_t isinlinealloc; // if this is allocated inline
// removed attributes
-    int32_t depth; // not a well-defined concept
-    int8_t isleaftype; // already deprecated, now gone

fix #25044
fix #23567
fix #17086
fix #25310

This still needs docs from #17086 (comment), and I should go back through and add some tests. But wanted to start by publish the functional changes now.

@vtjnash vtjnash added potential benchmark Could make a good benchmark in BaseBenchmarks needs tests Unit tests are required for this change needs docs Documentation for this change is required needs news A NEWS entry is required for this change labels Jan 10, 2018
else if (ty != to) {
unboxed = ctx.builder.CreateBitCast(unboxed, to);
unboxed = ctx.builder.CreateBitCast(unboxed, to);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

@JeffBezanson
Copy link
Sponsor Member

👍 Looks great!


_isleaftype(@nospecialize(t)) = (@_pure_meta; isa(t, DataType) && t.isleaftype)
_isleaftype(@nospecialize(t)) = (@_pure_meta; isa(t, DataType) && (t.isdispatchtuple || t.isconcretetype)) # deprecated
isdispatchtuple(@nospecialize(t)) = (@_pure_meta; isa(t, DataType) && t.isdispatchtuple)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs? It's exported.

@vtjnash vtjnash removed the needs docs Documentation for this change is required label Jan 12, 2018
end

"""
Base.isstructype(T) -> Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isprimitivetype

isstructtype(t::DataType) = (@_pure_meta; length(t.types) != 0 || (t.size==0 && !t.abstract))
isstructtype(x) = (@_pure_meta; false)
"""
Base.isstructype(T) -> Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isstructtype

@vtjnash vtjnash removed the needs news A NEWS entry is required for this change label Jan 13, 2018
@vtjnash vtjnash removed the needs tests Unit tests are required for this change label Jan 15, 2018
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Jan 15, 2018

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

@vtjnash vtjnash removed the potential benchmark Could make a good benchmark in BaseBenchmarks label Jan 15, 2018
@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@ararslan
Copy link
Member

Random moving to the stdlib broke Nanosoldier. I have a fix up, I just need to retune the benchmarks then I'll run Nanosoldier here when that's done.

@ararslan
Copy link
Member

@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
Sponsor Member Author

vtjnash commented Jan 16, 2018

Seems mostly noise, but let me run that again to see if any are consistently changed: @nanosoldier runbenchmarks(ALL, vs=":master")

@ararslan
Copy link
Member

@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
Sponsor Member Author

vtjnash commented Jan 17, 2018

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

@ararslan
Copy link
Member

@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/isleaftype-kill branch 4 times, most recently from b230e03 to 5cc69d1 Compare January 18, 2018 22:13
@vtjnash vtjnash merged commit 4f0b09e into master Jan 19, 2018
@vtjnash vtjnash deleted the jn/isleaftype-kill branch January 19, 2018 15:50
@JeffBezanson
Copy link
Sponsor Member

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants