Optimizer: Re-use CFG from type inference #50924

topolarity · 2023-08-15T15:31:00Z

This change will allow us to re-use Inference-collected information at the basic block level, such as the bb_vartables.

There were a couple of spots where the unreachable insertion pass at IRCode conversion (i.e. optimizer entry) was ignoring statically divergent code that inference had discovered:

%x = SlotNumber(3) can throw and cause the following statements to be statically unreachable
goto #b if not %cond can be statically throwing if %cond is known to never be Bool (or to always throw during its own evaluation)

CFG re-computation was hiding these bugs by flowing through the "Core.Const(...)"-wrapped statements that would follow, inserting unnecessary but harmless extra branches in the CFG.

gbaraldi · 2023-08-15T15:34:29Z

Potentially we could call adopt_thread on these threads but I would hold off on that until we figure some of the weird behaviour around them.

src/timing.c

base/compiler/optimize.jl

This change will allow us to re-use Inference-collected information at the basic block level, such as the `bb_vartables`. There were a couple of spots where the `unreachable` insertion pass at IRCode conversion (i.e. optimizer entry) was ignoring statically divergent code that inference had discovered: - `%x = SlotNumber(3)` can throw and cause the following statements to be statically unreachable - `goto #b if not %cond` can be statically throwing if %cond is known to never be Bool (or to always throw during its own evaluation) CFG re-computation was hiding these bugs by flowing through the "Core.Const(...)"-wrapped statements that would follow, inserting unnecessary but harmless extra branches in the CFG.

These checks are important to make sure that `verify_ir` does not end up triggering out-of-bounds errors as it checks IR. It also fills out some of the verification: - Confirms that cfg.index is correct - Confirms that cfg.blocks[i].stmts cover the complete IR

For some reason the recent CFG change caused us to generate: ``` 1-element Vector{Any}: CodeInfo( 1 ─ nothing::Nothing └── return true ) => Bool ``` instead of ``` 1-element Vector{Any}: CodeInfo( 1 ─ return true ) => Bool ``` but I believe this difference is harmless.

aviatesk

Looks very nice. Sorry for comments after merge, but we can address them later anyway.

aviatesk · 2023-08-19T11:18:49Z

base/compiler/typeinfer.jl

@@ -551,9 +551,9 @@ function finish(me::InferenceState, interp::AbstractInterpreter)
        # annotate fulltree with type information,
        # either because we are the outermost code, or we might use this later
        doopt = (me.cached || me.parent !== nothing)
-        recompute_cfg = type_annotate!(interp, me, doopt)
+        type_annotate!(interp, me, doopt)


We can eliminate anything related to any_unreachable within type_annotate! now.

aviatesk · 2023-08-19T11:23:57Z

base/compiler/optimize.jl

            idx += 1
            prevloc = codeloc
        end
-        if code[idx] isa Expr && ssavaluetypes[idx] === Union{}
+        if ssavaluetypes[idx] === Union{} && !(code[idx] isa Core.Const)


Maybe we want to set up a special and explicit marker type instead of Const to indicate a statement is known to be unreachable, e.g.

struct Unreachable stmt Unreachable(@nospecialize stmt) = new(stmt) end

Dependent on #50924 . This removes the last remaining usage of `TypedSlot`. As a bonus, this evicts all of the "tmerge" type-iteration that was being done in slot2ssa. --------- Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>

This is a partial back-port of JuliaLang#50924, where we discovered that the optimizer would ignore: 1. must-throw `%XX = SlotNumber(_)` statements 2. must-throw `goto #bb if not %x` statements when re-building the CFG during IRCode conversion. This is mostly harmless, except that in the case of (1) we can accidentally fall through the statically deleted (`Const()`-wrapped) code from inference and observe a control-flow edge that never should have existed, such as an edge into a catch block. Such an edge is invalid semantically and breaks our SSA conversion. This one-line change fixes (1) but not (2), which is enough for IR validity. We should follow-up with a tweak to `compute_basic_blocks` to enforce this requirement.

This is a partial back-port of JuliaLang#50924, where we discovered that the optimizer would ignore: 1. must-throw `%XX = SlotNumber(_)` statements 2. must-throw `goto #bb if not %x` statements when re-building the CFG during IRCode conversion. This is mostly harmless, except that in the case of (1) we can accidentally fall through the statically deleted (`Const()`-wrapped) code from inference and observe a control-flow edge that never should have existed, such as an edge into a catch block. Such an edge is invalid semantically and breaks our SSA conversion. This one-line change fixes (1) but not (2), which is enough for IR validity. We should follow-up with a tweak to `compute_basic_blocks` to enforce this requirement. The test added here is very brittle, but it's better than nothing until we have utilities to provide hand-written (pre-optimizer) IR and pass it through part of our pipeline.

This is a partial back-port of #50924, where we discovered that the optimizer would ignore: 1. must-throw `%XX = SlotNumber(_)` statements 2. must-throw `goto #bb if not %x` statements This is mostly harmless, except that in the case of (1) we can accidentally fall through the statically deleted (`Const()`-wrapped) code from inference and end up observing a control-flow edge that never existed. If the spurious edge is to a catch block, then the edge is invalid semantically and breaks our SSA conversion. This one-line change fixes (1) but not (2), which is enough for IR validity. Resolves part of #53366.

…53512) This is a partial back-port of JuliaLang#50924, where we discovered that the optimizer would ignore: 1. must-throw `%XX = SlotNumber(_)` statements 2. must-throw `goto #bb if not %x` statements This is mostly harmless, except that in the case of (1) we can accidentally fall through the statically deleted (`Const()`-wrapped) code from inference and end up observing a control-flow edge that never existed. If the spurious edge is to a catch block, then the edge is invalid semantically and breaks our SSA conversion. This one-line change fixes (1) but not (2), which is enough for IR validity. Resolves part of JuliaLang#53366. (cherry picked from commit 035d17a)

topolarity requested review from Keno and aviatesk August 15, 2023 15:31

vchuravy reviewed Aug 15, 2023

View reviewed changes

src/timing.c Outdated Show resolved Hide resolved

topolarity force-pushed the reuse-typeinference-cfg branch from 72e319c to 5c0b04a Compare August 15, 2023 17:13

brenhinkeller added the compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) label Aug 15, 2023

topolarity mentioned this pull request Aug 16, 2023

slot2ssa: remove TypedSlot #50943

Merged

2 tasks

Keno reviewed Aug 16, 2023

View reviewed changes

base/compiler/optimize.jl Show resolved Hide resolved

Keno reviewed Aug 16, 2023

View reviewed changes

base/compiler/optimize.jl Outdated Show resolved Hide resolved

Keno approved these changes Aug 16, 2023

View reviewed changes

topolarity added 3 commits August 17, 2023 18:07

topolarity force-pushed the reuse-typeinference-cfg branch from 5c0b04a to ce6b332 Compare August 17, 2023 22:13

Keno merged commit 49e6ff8 into JuliaLang:master Aug 18, 2023

aviatesk reviewed Aug 19, 2023

View reviewed changes

aviatesk mentioned this pull request Sep 7, 2023

remove special case for the no-op first statement in fully_eliminated #51227

Merged

topolarity mentioned this pull request Feb 28, 2024

Fix condition for unreachable code in IRCode conversion #53512

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimizer: Re-use CFG from type inference #50924

Optimizer: Re-use CFG from type inference #50924

topolarity commented Aug 15, 2023

gbaraldi commented Aug 15, 2023

aviatesk left a comment

aviatesk Aug 19, 2023

aviatesk Aug 19, 2023

Optimizer: Re-use CFG from type inference #50924

Optimizer: Re-use CFG from type inference #50924

Conversation

topolarity commented Aug 15, 2023

gbaraldi commented Aug 15, 2023

aviatesk left a comment

Choose a reason for hiding this comment

aviatesk Aug 19, 2023

Choose a reason for hiding this comment

aviatesk Aug 19, 2023

Choose a reason for hiding this comment