Skip to content

Commit ccd2746

Browse files
committed
optimizer: eliminate more isdefined checks
Follows up #44708 -- in that PR I missed the most obvious optimization opportunity, i.e. we can safely eliminate `isdefined` checks when all fields are defined at allocation site. This change allows us to eliminate capturing closure constructions when the body and callsite of capture closure is available within a optimized frame, e.g.: ```julia function abmult(r::Int, x0) if r < 0 r = -r end f = x -> x * r return @inline f(x0) end ``` ```diff diff --git a/_master.jl b/_pr.jl index ea06d865b75..c38f221090f 100644 --- a/_master.jl +++ b/_pr.jl @@ -1,24 +1,19 @@ julia> @code_typed abmult(-3, 3) CodeInfo( -1 ── %1 = Core.Box::Type{Core.Box} -│ %2 = %new(%1, r@_2)::Core.Box -│ %3 = Core.isdefined(%2, :contents)::Bool -└─── goto #3 if not %3 +1 ── goto #3 if not true 2 ── goto #4 3 ── $(Expr(:throw_undef_if_not, :r, false))::Any -4 ┄─ %7 = (r@_2 < 0)::Any -└─── goto #9 if not %7 -5 ── %9 = Core.isdefined(%2, :contents)::Bool -└─── goto #7 if not %9 +4 ┄─ %4 = (r@_2 < 0)::Any +└─── goto #9 if not %4 +5 ── goto #7 if not true 6 ── goto #8 7 ── $(Expr(:throw_undef_if_not, :r, false))::Any -8 ┄─ %13 = -r@_2::Any -9 ┄─ %14 = φ (#4 => r@_2, #8 => %13)::Any -│ %15 = Core.isdefined(%2, :contents)::Bool -└─── goto #11 if not %15 +8 ┄─ %9 = -r@_2::Any +9 ┄─ %10 = φ (#4 => r@_2, #8 => %9)::Any +└─── goto #11 if not true 10 ─ goto #12 11 ─ $(Expr(:throw_undef_if_not, :r, false))::Any -12 ┄ %19 = (x0 * %14)::Any +12 ┄ %14 = (x0 * %10)::Any └─── goto #13 -13 ─ return %19 +13 ─ return %14 ) => Any ```
1 parent 5dc6155 commit ccd2746

File tree

2 files changed

+35
-12
lines changed

2 files changed

+35
-12
lines changed

base/compiler/ssair/passes.jl

+10-12
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,7 @@ function lift_leaves(compact::IncrementalCompact,
372372
lift_arg!(compact, leaf, cache_key, def, 1+field, lifted_leaves)
373373
continue
374374
elseif isexpr(def, :new)
375-
typ = widenconst(types(compact)[leaf])
376-
if isa(typ, UnionAll)
377-
typ = unwrap_unionall(typ)
378-
end
375+
typ = unwrap_unionall(widenconst(types(compact)[leaf]))
379376
(isa(typ, DataType) && !isabstracttype(typ)) || return nothing
380377
@assert !ismutabletype(typ)
381378
if length(def.args) < 1+field
@@ -786,10 +783,7 @@ function sroa_pass!(ir::IRCode)
786783
push!(preserved, preserved_arg.id)
787784
continue
788785
elseif isexpr(def, :new)
789-
typ = widenconst(argextype(SSAValue(defidx), compact))
790-
if isa(typ, UnionAll)
791-
typ = unwrap_unionall(typ)
792-
end
786+
typ = unwrap_unionall(widenconst(argextype(SSAValue(defidx), compact)))
793787
if typ isa DataType && !ismutabletype(typ)
794788
record_immutable_preserve!(new_preserves, def, compact)
795789
push!(preserved, preserved_arg.id)
@@ -948,10 +942,7 @@ function sroa_mutables!(ir::IRCode, defuses::IdDict{Int, Tuple{SPCSet, SSADefUse
948942
defexpr = ir[SSAValue(idx)][:inst]
949943
isexpr(defexpr, :new) || continue
950944
newidx = idx
951-
typ = ir.stmts[newidx][:type]
952-
if isa(typ, UnionAll)
953-
typ = unwrap_unionall(typ)
954-
end
945+
typ = unwrap_unionall(ir.stmts[newidx][:type])
955946
# Could still end up here if we tried to setfield! on an immutable, which would
956947
# error at runtime, but is not illegal to have in the IR.
957948
ismutabletype(typ) || continue
@@ -1023,6 +1014,13 @@ function sroa_mutables!(ir::IRCode, defuses::IdDict{Int, Tuple{SPCSet, SSADefUse
10231014
end
10241015
has_safe_def(ir, get_domtree(), allblocks, du, newidx, use.idx) || @goto skip
10251016
end
1017+
else # always have some definition at the allocation site
1018+
for i = 1:length(du.uses)
1019+
use = du.uses[i]
1020+
if use.kind === :isdefined
1021+
ir[SSAValue(use.idx)][:inst] = true
1022+
end
1023+
end
10261024
end
10271025
end
10281026
# Everything accounted for. Go field by field and perform idf:

test/compiler/irpasses.jl

+25
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,31 @@ let src = code_typed1(isdefined_elim)
493493
end
494494
@test isdefined_elim() == Any[]
495495

496+
function abmult(r::Int, x0)
497+
if r < 0
498+
r = -r
499+
end
500+
f = x -> x * r
501+
return @inline f(x0)
502+
end
503+
let src = code_typed1(abmult, (Int,Int))
504+
@test is_scalar_replaced(src)
505+
end
506+
@test abmult(-3, 3) == 9
507+
508+
function abmult2(r0::Int, x0)
509+
r::Int = r0
510+
if r < 0
511+
r = -r
512+
end
513+
f = x -> x * r
514+
return f(x0)
515+
end
516+
let src = code_typed1(abmult2, (Int,Int))
517+
@test is_scalar_replaced(src)
518+
end
519+
@test abmult2(-3, 3) == 9
520+
496521
# comparison lifting
497522
# ==================
498523

0 commit comments

Comments
 (0)