Skip to content

Commit

Permalink
inference: fix a mistake with ssa_def_expr usage (JuliaLang#30179)
Browse files Browse the repository at this point in the history
Uses of this function previously assumed that all Slots were SSA.
Fortunately, we now just have one user (post JuliaLang#29935) and it has very simple requirements
so we can check conservatively for this case without loss of (much) accuracy.
  • Loading branch information
vtjnash authored Nov 29, 2018
1 parent a50d5af commit 2ec5560
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 7 deletions.
27 changes: 20 additions & 7 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,22 @@ end

# This is only for use with `Conditional`.
# In general, usage of this is wrong.
function ssa_def_expr(@nospecialize(arg), sv::InferenceState)
function ssa_def_slot(@nospecialize(arg), sv::InferenceState)
init = sv.currpc
while isa(arg, SSAValue)
arg = sv.src.code[arg.id]
init = arg.id
arg = sv.src.code[init]
end
arg isa SlotNumber || return nothing
for i = init:(sv.currpc - 1)
# conservatively make sure there isn't potentially another conflicting assignment to
# the same slot between the def and usage
# we can assume the IR is sorted, since the front-end only creates SSA values in order
e = sv.src.code[i]
e isa Expr || continue
if e.head === :(=) && e.args[1] === arg
return nothing
end
end
return arg
end
Expand Down Expand Up @@ -552,8 +565,8 @@ function abstract_call(@nospecialize(f), fargs::Union{Tuple{},Vector{Any}}, argt
cnd = argtypes[2]::Conditional
tx = argtypes[3]
ty = argtypes[4]
a = ssa_def_expr(fargs[3], sv)
b = ssa_def_expr(fargs[4], sv)
a = ssa_def_slot(fargs[3], sv)
b = ssa_def_slot(fargs[4], sv)
if isa(a, Slot) && slot_id(cnd.var) == slot_id(a)
tx = typeintersect(tx, cnd.vtype)
end
Expand All @@ -572,7 +585,7 @@ function abstract_call(@nospecialize(f), fargs::Union{Tuple{},Vector{Any}}, argt
elseif (rt === Bool || (isa(rt, Const) && isa(rt.val, Bool))) && isa(fargs, Vector{Any})
# perform very limited back-propagation of type information for `is` and `isa`
if f === isa
a = ssa_def_expr(fargs[2], sv)
a = ssa_def_slot(fargs[2], sv)
if isa(a, Slot)
aty = widenconst(argtypes[2])
if rt === Const(false)
Expand All @@ -591,8 +604,8 @@ function abstract_call(@nospecialize(f), fargs::Union{Tuple{},Vector{Any}}, argt
end
end
elseif f === (===)
a = ssa_def_expr(fargs[2], sv)
b = ssa_def_expr(fargs[3], sv)
a = ssa_def_slot(fargs[2], sv)
b = ssa_def_slot(fargs[3], sv)
aty = argtypes[2]
bty = argtypes[3]
# if doing a comparison to a singleton, consider returning a `Conditional` instead
Expand Down
9 changes: 9 additions & 0 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1615,6 +1615,15 @@ g26172(::Val{0}) = ()
g26172(v) = (nothing, g26172(f26172(v))...)
@test @inferred(g26172(Val(10))) === ntuple(_ -> nothing, 10)

function conflicting_assignment_conditional()
x = iterate([])
if x === (x = 4; nothing)
return x
end
return 5
end
@test @inferred(conflicting_assignment_conditional()) === 4

# 26826 constant prop through varargs

struct Foo26826{A,B}
Expand Down

0 comments on commit 2ec5560

Please sign in to comment.