adce_pass: Drop phinode edges that can be proved unused #43922

Keno · 2022-01-25T07:17:34Z

Review note: Base branch is #43227, since that's needed for the test to pass. I'd suggest merging that first, which will automatically update the base branch.

Consider a case like:

f(x::Float64) = println(x)
f(x::SomeBigStruct) = nothing

Union splitting introduces code that looks like:

   goto 2 if not ...
1: a = ::Float64
   goto 3
2: b = new(SomeBigStruct, ...)::SomeBigStruct
3: c = phi(a, b)
   if !isa(c, Float64) goto 5
4: d = PiNode(c, Float64)
   println(d)
   goto 6
5: nothing
6: return nothing

Now, #43227 will turn this into:

   goto 2 if not ...
1: a = ::Float64
   goto 3
2: b = new(SomeBigStruct, ...)::SomeBigStruct
3: c = phi(a, b)
   cond = phi(true, false)
   if !cond goto 5
4: d = PiNode(c, Float64)
   println(d)
   goto 6
5: nothing
6: return nothing

But even though dynamically b is never used,
it doesn't get deleted, because there's still a
use in the PhiNode c.

This PR teaches adce to recognize this situation and,
for PhiNodes that are only ever used by PiNodes, drop
any edges that are known to be unused. E.g. in the above
case, the adce improvements would turn it into:

   goto 2 if not ...
1: a = ::Float64
   goto 3
2: b = new(SomeBigStruct, ...)::SomeBigStruct
3: c = phi(a)
   cond = phi(true, false)
   if !cond goto 5
4: d = PiNode(c, Float64)
   println(d)
   goto 6
5: nothing
6: return nothing

Which in turn would let regular DCE drop the allocation:

   goto 2 if not ...
1: a = ::Float64
   goto 3
2: nothing
3: c = phi(a)
   cond = phi(true, false)
   if !cond goto 5
4: d = PiNode(c, Float64)
   println(d)
   goto 6
5: nothing
6: return nothing

Co-authored-by: Ian Atol [email protected]

This commit implements more comparison liftings. Especially, this change enables the compiler to lift `isa`/`isdefined` checks (i.e. replace a comparison call with ϕ-node by CFG union-splitting). For example, the code snippet below will run 500x faster: ```julia function compute(n) s = 0 itr = 1:n st = iterate(itr) while isdefined(st, 2) # mimic our iteration protocol with `isdefined` v, st = st s += v st = iterate(itr, st) end s end ``` Although it seems like the codegen for `isa` is fairly optimized already and so I could not find any performance benefit for `isa`-lifting (`code_llvm` emits mostly equivalent code), but I hope it's more ideal if we can do the equivalent optimization on Julia level so that we can just consult to `code_typed` for performance optimization.

Union splitting introduces patterns like: Consider a case like: ``` f(x::Float64) = println(x) f(x::SomeBigStruct) = nothing ``` ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) if !isa(c, Float64) goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Now, #43227 will turn this into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` But even though dynamically `b` is never used, it doesn't get deleted, because there's still a use in the PhiNode `c`. This PR teaches adce to recognize this situation and, for PhiNodes that are only ever used by PiNodes, drop any edges that are known to be unused. E.g. in the above case, the adce improvements would turn it into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Which in turn would let regular DCE drop the allocation: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: nothing 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Co-authored-by: Ian Atol <[email protected]>

aviatesk

Should be a good enhancement.

test/compiler/irpasses.jl

base/compiler/ssair/passes.jl

aviatesk · 2022-01-25T08:02:47Z

base/compiler/ssair/passes.jl

+ if phi_uses[phi] != 0
+ continue
+ end
+ if t === Union{}


When this condition can be hit? Do we sometimes have π(...)::Union{}?

Yeah, or during compact all the uses turned out dead or something. We could do this folding in compact, but since this is the place that changes it, might as well do it here.

aviatesk · 2022-01-25T08:04:18Z

base/compiler/ssair/passes.jl

 end
 end
 end
 non_dce_finish!(compact)
 for phi in all_phis
 count_uses(compact.result[phi][:inst]::PhiNode, phi_uses)
 end
+ # Narrow any union phi nodes that have unused branches
+ for (phi, t) in zip(unionphis, unionphi_types)


I remember @vtjnash said that this (phi, t) may allocate arbitrary tuple types depending on user types fed into unionphi_types:

Suggested change

for (phi, t) in zip(unionphis, unionphi_types)

@assert length(unionphis) == length(unionphi_types)

for i = length(unionphis)

phi = unionphis[i]

t = unionphi_types[i]

Ah, right. Good catch.

Co-authored-by: Shuhei Kadowaki <[email protected]>

vtjnash · 2022-01-26T17:22:03Z

base/compiler/ssair/passes.jl

+ deleteat!(unionphis, first(r))
+ deleteat!(unionphi_types, first(r))


Seems like you should be using a Set or small (unsorted) array, if you need deletion. Otherwise this object is costing O(n)

vtjnash · 2022-01-26T17:23:20Z

LGTM

Just a slight tweak to #43922 to also allow the implicit narrowing from other phi users. It's not a particularly common occurrence, but can happen particularly when running the adce pass twice, (because that narrows the types of some phis by dropping edges).

) * optimizer: lift more comparisons This commit implements more comparison liftings. Especially, this change enables the compiler to lift `isa`/`isdefined` checks (i.e. replace a comparison call with ϕ-node by CFG union-splitting). For example, the code snippet below will run 500x faster: ```julia function compute(n) s = 0 itr = 1:n st = iterate(itr) while isdefined(st, 2) # mimic our iteration protocol with `isdefined` v, st = st s += v st = iterate(itr, st) end s end ``` Although it seems like the codegen for `isa` is fairly optimized already and so I could not find any performance benefit for `isa`-lifting (`code_llvm` emits mostly equivalent code), but I hope it's more ideal if we can do the equivalent optimization on Julia level so that we can just consult to `code_typed` for performance optimization. * adce_pass: Drop phinode uses that can be proved unused Union splitting introduces patterns like: Consider a case like: ``` f(x::Float64) = println(x) f(x::SomeBigStruct) = nothing ``` ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) if !isa(c, Float64) goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Now, JuliaLang#43227 will turn this into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` But even though dynamically `b` is never used, it doesn't get deleted, because there's still a use in the PhiNode `c`. This PR teaches adce to recognize this situation and, for PhiNodes that are only ever used by PiNodes, drop any edges that are known to be unused. E.g. in the above case, the adce improvements would turn it into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Which in turn would let regular DCE drop the allocation: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: nothing 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Co-authored-by: Shuhei Kadowaki <[email protected]> Co-authored-by: Ian Atol <[email protected]>

Just a slight tweak to JuliaLang#43922 to also allow the implicit narrowing from other phi users. It's not a particularly common occurrence, but can happen particularly when running the adce pass twice, (because that narrows the types of some phis by dropping edges).

) * optimizer: lift more comparisons This commit implements more comparison liftings. Especially, this change enables the compiler to lift `isa`/`isdefined` checks (i.e. replace a comparison call with ϕ-node by CFG union-splitting). For example, the code snippet below will run 500x faster: ```julia function compute(n) s = 0 itr = 1:n st = iterate(itr) while isdefined(st, 2) # mimic our iteration protocol with `isdefined` v, st = st s += v st = iterate(itr, st) end s end ``` Although it seems like the codegen for `isa` is fairly optimized already and so I could not find any performance benefit for `isa`-lifting (`code_llvm` emits mostly equivalent code), but I hope it's more ideal if we can do the equivalent optimization on Julia level so that we can just consult to `code_typed` for performance optimization. * adce_pass: Drop phinode uses that can be proved unused Union splitting introduces patterns like: Consider a case like: ``` f(x::Float64) = println(x) f(x::SomeBigStruct) = nothing ``` ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) if !isa(c, Float64) goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Now, JuliaLang#43227 will turn this into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a, b) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` But even though dynamically `b` is never used, it doesn't get deleted, because there's still a use in the PhiNode `c`. This PR teaches adce to recognize this situation and, for PhiNodes that are only ever used by PiNodes, drop any edges that are known to be unused. E.g. in the above case, the adce improvements would turn it into: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: b = new(SomeBigStruct, ...)::SomeBigStruct 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Which in turn would let regular DCE drop the allocation: ``` goto 2 if not ... 1: a = ::Float64 goto 3 2: nothing 3: c = phi(a) cond = phi(true, false) if !cond goto 5 4: d = PiNode(c, Float64) println(d) goto 6 5: nothing 6: return nothing ``` Co-authored-by: Shuhei Kadowaki <[email protected]> Co-authored-by: Ian Atol <[email protected]>

Just a slight tweak to JuliaLang#43922 to also allow the implicit narrowing from other phi users. It's not a particularly common occurrence, but can happen particularly when running the adce pass twice, (because that narrows the types of some phis by dropping edges).

aviatesk and others added 2 commits January 25, 2022 16:00

Keno requested a review from aviatesk January 25, 2022 07:17

aviatesk reviewed Jan 25, 2022

View reviewed changes

Keno and others added 2 commits January 25, 2022 03:05

Apply suggestions from code review

d556dd6

Co-authored-by: Shuhei Kadowaki <[email protected]>

Make sure not to allocate useless tuples

561a297

Base automatically changed from avi/liftcmps to master January 25, 2022 08:47

Merge branch 'master' into HEAD

1aa4762

aviatesk approved these changes Jan 25, 2022

View reviewed changes

aviatesk added the status:merge me PR is reviewed. Merge when all tests are passing label Jan 25, 2022

fixup!: this test needs to run at top-level (to run the optimization)

a312321

Keno merged commit b34cb17 into master Jan 26, 2022

Keno deleted the kf/ia/adce_piuseonly branch January 26, 2022 03:57

aviatesk mentioned this pull request Jan 26, 2022

NFC: minor refactors on compiler/passes.jl #43928

Merged

vtjnash reviewed Jan 26, 2022

View reviewed changes

Keno mentioned this pull request Jan 27, 2022

adce: Slight tweak to phinode elimination #43948

Merged

DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Jan 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adce_pass: Drop phinode edges that can be proved unused #43922

adce_pass: Drop phinode edges that can be proved unused #43922

Keno commented Jan 25, 2022

aviatesk left a comment

aviatesk Jan 25, 2022

Keno Jan 25, 2022

aviatesk Jan 25, 2022

Keno Jan 25, 2022

vtjnash Jan 26, 2022

vtjnash commented Jan 26, 2022

- for (phi, t) in zip(unionphis, unionphi_types)
+ @assert length(unionphis) == length(unionphi_types)
+ for i = length(unionphis)
+ phi = unionphis[i]
+ t = unionphi_types[i]

		deleteat!(unionphis, first(r))
		deleteat!(unionphi_types, first(r))

adce_pass: Drop phinode edges that can be proved unused #43922

adce_pass: Drop phinode edges that can be proved unused #43922

Conversation

Keno commented Jan 25, 2022

aviatesk left a comment

Choose a reason for hiding this comment

aviatesk Jan 25, 2022

Choose a reason for hiding this comment

Keno Jan 25, 2022

Choose a reason for hiding this comment

aviatesk Jan 25, 2022

Choose a reason for hiding this comment

Keno Jan 25, 2022

Choose a reason for hiding this comment

vtjnash Jan 26, 2022

Choose a reason for hiding this comment

vtjnash commented Jan 26, 2022