Skip to content

Commit 981b8fc

Browse files
committed
[NewOptimizer] Make stmt_effect_free less aggressive
Previously, the new optimizer used the pre-existing `effect_free` function to determine whether it is safe to remove an unreferenced statement. However, this function ignores many builtin's error cases, causing them to be removed when that is not legal to do (because that would potentially remove an exception that would otherwise be thrown). Start fixing this, by introducing a version of the function that is correct for a subset of intresting functions. We will likely need to expand this when we look at the benchmarks, but this should be correct for now.
1 parent 23388ae commit 981b8fc

File tree

3 files changed

+165
-13
lines changed

3 files changed

+165
-13
lines changed

base/compiler/ssair/passes.jl

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,11 @@ struct SSADefUse
1717
end
1818
SSADefUse() = SSADefUse(Int[], Int[], Int[])
1919

20-
function try_compute_fieldidx(@nospecialize(typ), @nospecialize(use_expr))
20+
function try_compute_fieldidx_expr(@nospecialize(typ), @nospecialize(use_expr))
2121
field = use_expr.args[3]
2222
isa(field, QuoteNode) && (field = field.value)
2323
isa(field, Union{Int, Symbol}) || return nothing
24-
if isa(field, Symbol)
25-
field = fieldindex(typ, field, false)
26-
field == 0 && return nothing
27-
elseif isa(field, Integer)
28-
(1 <= field <= fieldcount(typ)) || return nothing
29-
end
30-
return field
24+
return try_compute_fieldidx(typ, field)
3125
end
3226

3327
function lift_defuse(cfg::CFG, ssa::SSADefUse)
@@ -280,15 +274,15 @@ function getfield_elim_pass!(ir::IRCode, domtree::DomTree)
280274
union!(mid, intermediaries)
281275
continue
282276
end
283-
field = try_compute_fieldidx(typ, stmt)
277+
field = try_compute_fieldidx_expr(typ, stmt)
284278
field === nothing && continue
285279
forwarded = def.args[1+field]
286280
else
287281
obj = compact_exprtype(compact, def)
288282
isa(obj, Const) || continue
289283
obj = obj.val
290284
isimmutable(obj) || continue
291-
field = try_compute_fieldidx(typeof(obj), stmt)
285+
field = try_compute_fieldidx_expr(typeof(obj), stmt)
292286
field === nothing && continue
293287
isdefined(obj, field) || continue
294288
val = getfield(obj, field)
@@ -330,13 +324,13 @@ function getfield_elim_pass!(ir::IRCode, domtree::DomTree)
330324
fielddefuse = SSADefUse[SSADefUse() for _ = 1:fieldcount(typ)]
331325
ok = true
332326
for use in defuse.uses
333-
field = try_compute_fieldidx(typ, ir[SSAValue(use)])
327+
field = try_compute_fieldidx_expr(typ, ir[SSAValue(use)])
334328
field === nothing && (ok = false; break)
335329
push!(fielddefuse[field].uses, use)
336330
end
337331
ok || continue
338332
for use in defuse.defs
339-
field = try_compute_fieldidx(typ, ir[SSAValue(use)])
333+
field = try_compute_fieldidx_expr(typ, ir[SSAValue(use)])
340334
field === nothing && (ok = false; break)
341335
push!(fielddefuse[field].defs, use)
342336
end

base/compiler/ssair/queries.jl

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,57 @@
1+
"""
2+
Determine whether a statement is side-effect-free, i.e. may be removed if it has no uses.
3+
"""
14
function stmt_effect_free(@nospecialize(stmt), src, mod::Module)
25
isa(stmt, Union{PiNode, PhiNode}) && return true
36
isa(stmt, Union{ReturnNode, GotoNode, GotoIfNot}) && return false
4-
return effect_free(stmt, src, mod, true)
7+
isa(stmt, GlobalRef) && isdefined(stmt.mod, stmt.name)
8+
(isa(stmt, Symbol) || isa(stmt, SSAValue) || isa(stmt, Argument)) && return true
9+
isa(stmt, Slot) && return false # Slots shouldn't occur in the IR at this point, but let's be defensive here
10+
if isa(stmt, Expr)
11+
e = stmt::Expr
12+
head = e.head
13+
is_meta_expr_head(head) && return true
14+
if head === :static_parameter
15+
# if we aren't certain enough about the type, it might be an UndefVarError at runtime
16+
return isa(e.typ, Const) || issingletontype(widenconst(e.typ))
17+
end
18+
(e.typ === Bottom) && return false
19+
ea = e.args
20+
if head === :call
21+
f = exprtype(ea[1], src, mod)
22+
if isa(f, Const)
23+
f = f.val
24+
elseif isType(f)
25+
f = f.parameters[1]
26+
else
27+
return false
28+
end
29+
f === return_type && return true
30+
# TODO: This needs significant refinement
31+
contains_is(_PURE_BUILTINS, f) || return false
32+
return builtin_nothrow(f, Any[exprtype(ea[i], src, mod) for i = 2:length(ea)])
33+
elseif head === :new
34+
a = ea[1]
35+
typ = exprtype(a, src, mod)
36+
# `Expr(:new)` of unknown type could raise arbitrary TypeError.
37+
typ, isexact = instanceof_tfunc(typ)
38+
isexact || return false
39+
isconcretedispatch(typ) || return false
40+
typ = typ::DataType
41+
fieldcount(typ) >= length(ea) - 1 || return false
42+
for fld_idx in 1:(length(ea) - 1)
43+
eT = exprtype(ea[fld_idx + 1], src, mod)
44+
fT = fieldtype(typ, fld_idx)
45+
eT fT || return false
46+
end
47+
return true
48+
elseif head === :isdefined || head === :the_exception || head === :copyast
49+
return true
50+
else
51+
return false
52+
end
53+
end
54+
return true
555
end
656

757
function abstract_eval_ssavalue(s::SSAValue, src::IRCode)

base/compiler/tfuncs.jl

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,75 @@ function const_datatype_getfield_tfunc(sv, fld)
433433
return nothing
434434
end
435435

436+
function try_compute_fieldidx(@nospecialize(typ), @nospecialize(field))
437+
if isa(field, Symbol)
438+
field = fieldindex(typ, field, false)
439+
field == 0 && return nothing
440+
elseif isa(field, Integer)
441+
(1 <= field <= fieldcount(typ)) || return nothing
442+
else
443+
return nothing
444+
end
445+
return field
446+
end
447+
448+
function getfield_nothrow(argtypes::Vector{Any})
449+
2 <= length(argtypes) <= 3 || return false
450+
length(argtypes) == 2 && return getfield_nothrow(argtypes[1], argtypes[2], Const(true))
451+
return getfield_nothrow(argtypes[1], argtypes[2], argtypes[3])
452+
end
453+
function getfield_nothrow(@nospecialize(s00), @nospecialize(name), @nospecialize(inbounds))
454+
bounds_check_disabled = isa(inbounds, Const) && inbounds.val === false
455+
# If we don't have invounds and don't know the field, don't even bother
456+
if !bounds_check_disabled
457+
isa(name, Const) || return false
458+
end
459+
460+
# If we have s00 being a const, we can potentially refine our type-based analysis above
461+
if isa(s00, Const) || isconstType(s00)
462+
if !isa(s00, Const)
463+
sv = s00.parameters[1]
464+
else
465+
sv = s00.val
466+
end
467+
if isa(name, Const)
468+
(isa(sv, Module) && isa(name.val, Symbol)) || return false
469+
(isa(name.val, Symbol) || isa(name.val, Int)) || return false
470+
return isdefined(sv, name.val)
471+
end
472+
if bounds_check_disabled && !isa(sv, Module)
473+
# If bounds checking is disabled and all fields are assigned,
474+
# we may assume that we don't throw
475+
for i = 1:fieldcount(typeof(sv))
476+
isdefined(sv, i) || return false
477+
end
478+
return true
479+
end
480+
return false
481+
end
482+
483+
s = unwrap_unionall(widenconst(s00))
484+
if isa(s, Union)
485+
return getfield_nothrow(rewrap(s.a, s00), name, inbounds) &&
486+
getfield_nothrow(rewrap(s.b, s00), name, inbounds)
487+
elseif isa(s, Conditional)
488+
return false
489+
elseif isa(s, DataType)
490+
# Can't say anything about abstract types
491+
s.abstract && return false
492+
# If all fields are always initialized, and bounds check is disabled, we can assume
493+
# we don't throw
494+
(fieldcount(s) == s.ninitialized) && return true
495+
# Else we need to know what the field is
496+
isa(name, Const) || return false
497+
field = try_compute_fieldidx(s, name.val)
498+
field === nothing && return false
499+
field <= s.ninitialized && return true
500+
end
501+
502+
return false
503+
end
504+
436505
getfield_tfunc(@nospecialize(s00), @nospecialize(name), @nospecialize(inbounds)) =
437506
getfield_tfunc(s00, name)
438507
function getfield_tfunc(@nospecialize(s00), @nospecialize(name))
@@ -770,6 +839,45 @@ function tuple_tfunc(@nospecialize(argtype))
770839
return argtype
771840
end
772841

842+
function array_builtin_common_nothrow(argtypes, first_idx_idx)
843+
length(argtypes) >= 4 || return false
844+
(argtypes[0] Bool && argtypes[1] Array) || return false
845+
for i = first_idx_idx:length(argtypes)
846+
argtypes[i] Int || return false
847+
end
848+
# If we have @inbounds (first argument is false), we're allowed to assume we don't throw
849+
(isa(argtypes[0], Const) && !argtypes[0].val) && return true
850+
# Else we can't really say anything here
851+
# TODO: In the future we may be able to track the shapes of arrays though
852+
# inference.
853+
return false
854+
end
855+
856+
# Query whether the given builtin is guaranteed not to throw given the argtypes
857+
function builtin_nothrow(@nospecialize(f), argtypes::Array{Any,1})
858+
(f === tuple || f === svec) && return true
859+
if f === arrayset
860+
array_builtin_common_nothrow(argtypes, 4) || return true
861+
# Additionally check element type compatibility
862+
a = widenconst(argtypes[2])
863+
# Check that we can determine the element type
864+
(isa(a, DataType) && (isa(a.parameters[1], Type) || isa(a.parameters[1], TypeVar))) || return false
865+
at = a.parameters[1]
866+
# Check that the element type is compatible with the element we're assigning
867+
(argtypes[3] (isa(at, Type) ? at : at.lb)) || return false
868+
return true
869+
elseif f === arrayref
870+
return array_builtin_common_nothrow(argtypes, 3)
871+
elseif f === Core._expr
872+
return length(argtypes) >= 1 && argtypes[1] Symbol
873+
elseif f === invoke
874+
return false
875+
elseif f === getfield
876+
return getfield_nothrow(argtypes)
877+
end
878+
return false
879+
end
880+
773881
function builtin_tfunction(@nospecialize(f), argtypes::Array{Any,1},
774882
sv::Union{InferenceState,Nothing}, params::Params = sv.params)
775883
isva = !isempty(argtypes) && isvarargtype(argtypes[end])

0 commit comments

Comments
 (0)