Type inference opt passes clean up #18260
Conversation
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
b52f5f7
to
cc1c3e0
Compare
| function meta_elim_pass!(linfo::LambdaInfo, code::Array{Any,1}) | ||
| # 1. Remove place holders | ||
| # | ||
| # 2. If coverage is off, remove line number nodes that doesn't mark any |
e82a28b
to
f2b94c6
Compare
|
comment? |
| return false | ||
| elseif isa(ex, Slot) | ||
| return linfo.slotflags[(ex::Slot).id] & Slot_UsedUndef != 0 | ||
| elseif (isa(ex, Expr) || isa(ex, GotoNode) || isa(ex, LineNumberNode) || |
There was a problem hiding this comment.
Can we also remove GlobalRefs that are known to be defined?
There was a problem hiding this comment.
Sure and done.
I somehow thought it was already handled in type inference but seems that I was testing the wrong thing....
Consistently ignore line numbers and metadata nodes in all cases.
Move unexported `jl_sym_t*` GVs from `julia.h` to `julia_internal.h`
|
LGTM! |
Also collapse redundant debug info when coverage is off. Fix incorrect `Expr(:inbounds, false)` elimination.
f2b94c6
to
661235a
Compare
|
I've rebased this for a few time and it seems that there's always a saving in the sysimg size although the percentage can be as small as 0.5% or as big as 3%.... |
|
@nanosoldier |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
|
This looks good to me. The only issue I have is that it is now rather opaque as to where bounds check elimination happens. i.e. I wouldn't intuitively guess that I should look at the "meta elimination pass" to also find bounds check elimination. Perhaps that can be fixed with a better name for the function |
|
Is this still backportable if it proves stable or have we diverged too much already? |
|
This is mainly a clean up for #18196 and both should be backportable after tested and if really wanted. |
|
In case this is going to be backported, #18414 should be too. |
These are the parts that are not related to coverage from #18196. The changes included in the PR are,
propagate_inboundstoLambdaInfoExpr(:inbounds, false)(This increases the AST size by a little since some of the nodes were incorrectly deleted.)Overall this shrink the rodata size by 1-2%.
The last commit is some debugging/verifying code and will be removed before merging.
@nanosoldier
runbenchmarks(ALL, vs = ":master")