Skip to content

Commit 73d4666

Browse files
Kenoaviatesk
authored andcommitted
inference: Remove special casing for !
We have a handful of cases in inference where we look up functions by name (using `istopfunction`) and give them special behavior. I'd like to remove these. They're not only aesthetically ugly, but because they depend on binding lookups, rather than values, they have unclear semantics as those bindings change. They are also unsound should a user use the same name for something different in their own top modules (of course, it's unlikely that a user would do such a thing, but it's bad that they can't). This particular PR removes the special case for `!`, which was there to strengthen the inference result for `!` on Conditional. However, with a little bit of strengthening of the rest of the system, this can be equally well evaluated through the existing InterConditional mechanism.
1 parent 125bac4 commit 73d4666

File tree

3 files changed

+45
-9
lines changed

3 files changed

+45
-9
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,25 @@ call_result_unused(sv::InferenceState, currpc::Int) =
1313
isexpr(sv.src.code[currpc], :call) && isempty(sv.ssavalue_uses[currpc])
1414
call_result_unused(si::StmtInfo) = !si.used
1515

16+
is_const_bool_or_bottom(@nospecialize(b)) = (isa(b, Const) && isa(b.val, Bool)) || b == Bottom
17+
function can_propagate_conditional(@nospecialize(rt), argtypes::Vector{Any})
18+
return isa(rt, InterConditional) && isa(argtypes[rt.slot], Conditional) &&
19+
is_const_bool_or_bottom(rt.thentype) && is_const_bool_or_bottom(rt.thentype)
20+
end
21+
22+
function propagate_conditional(rt::InterConditional, cond::Conditional)
23+
new_thentype = rt.thentype === Const(false) ? cond.elsetype : cond.thentype
24+
new_elsetype = rt.elsetype === Const(true) ? cond.thentype : cond.elsetype
25+
if rt.thentype == Bottom
26+
@assert rt.elsetype != Bottom
27+
return Conditional(cond.slot, Bottom, new_elsetype)
28+
elseif rt.elsetype == Bottom
29+
@assert rt.thentype != Bottom
30+
return Conditional(cond.slot, new_thentype, Bottom)
31+
end
32+
return Conditional(cond.slot, new_thentype, new_elsetype)
33+
end
34+
1635
function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
1736
arginfo::ArgInfo, si::StmtInfo, @nospecialize(atype),
1837
sv::AbsIntState, max_methods::Int)
@@ -156,6 +175,15 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f),
156175
end
157176
@assert !(this_conditional isa Conditional || this_rt isa MustAlias) "invalid lattice element returned from inter-procedural context"
158177
seen += 1
178+
179+
if can_propagate_conditional(this_conditional, argtypes)
180+
# The only case where we need to keep this in rt is where
181+
# we can directly propagate the conditional to a slot argument
182+
# that is not one of our arguments, otherwise we keep all the
183+
# relevant information in `conditionals` below.
184+
this_rt = this_conditional
185+
end
186+
159187
rettype = rettype ₚ this_rt
160188
exctype = exctype ₚ this_exct
161189
if has_conditional(𝕃ₚ, sv) && this_conditional !== Bottom && is_lattice_bool(𝕃ₚ, rettype) && fargs !== nothing
@@ -409,6 +437,9 @@ function from_interconditional(𝕃ᵢ::AbstractLattice, @nospecialize(rt), sv::
409437
has_conditional(𝕃ᵢ, sv) || return widenconditional(rt)
410438
(; fargs, argtypes) = arginfo
411439
fargs === nothing && return widenconditional(rt)
440+
if can_propagate_conditional(rt, argtypes)
441+
return propagate_conditional(rt, argtypes[rt.slot]::Conditional)
442+
end
412443
slot = 0
413444
alias = nothing
414445
thentype = elsetype = Any
@@ -2217,13 +2248,6 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
22172248
end
22182249
elseif is_return_type(f)
22192250
return return_type_tfunc(interp, argtypes, si, sv)
2220-
elseif la == 2 && istopfunction(f, :!)
2221-
# handle Conditional propagation through !Bool
2222-
aty = argtypes[2]
2223-
if isa(aty, Conditional)
2224-
call = abstract_call_gf_by_type(interp, f, ArgInfo(fargs, Any[Const(f), Bool]), si, Tuple{typeof(f), Bool}, sv, max_methods) # make sure we've inferred `!(::Bool)`
2225-
return CallMeta(Conditional(aty.slot, aty.elsetype, aty.thentype), Any, call.effects, call.info)
2226-
end
22272251
elseif la == 3 && istopfunction(f, :!==)
22282252
# mark !== as exactly a negated call to ===
22292253
call = abstract_call_gf_by_type(interp, f, ArgInfo(fargs, Any[Const(f), Any, Any]), si, Tuple{typeof(f), Any, Any}, sv, max_methods)
@@ -3194,7 +3218,7 @@ function update_bestguess!(interp::AbstractInterpreter, frame::InferenceState,
31943218
# narrow representation of bestguess slightly to prepare for tmerge with rt
31953219
if rt isa InterConditional && bestguess isa Const
31963220
slot_id = rt.slot
3197-
old_id_type = slottypes[slot_id]
3221+
old_id_type = widenconditional(slottypes[slot_id])
31983222
if bestguess.val === true && rt.elsetype !== Bottom
31993223
bestguess = InterConditional(slot_id, old_id_type, Bottom)
32003224
elseif bestguess.val === false && rt.thentype !== Bottom

base/compiler/inferencestate.jl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,9 @@ mutable struct InferenceState
312312
nargtypes = length(argtypes)
313313
for i = 1:nslots
314314
argtyp = (i > nargtypes) ? Bottom : argtypes[i]
315+
if argtyp === Bool && has_conditional(typeinf_lattice(interp))
316+
argtyp = Conditional(i, Const(true), Const(false))
317+
end
315318
slottypes[i] = argtyp
316319
bb_vartable1[i] = VarState(argtyp, i > nargtypes)
317320
end

base/compiler/tfuncs.jl

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,19 @@ end
227227
@nospecs shift_tfunc(𝕃::AbstractLattice, x, y) = shift_tfunc(widenlattice(𝕃), x, y)
228228
@nospecs shift_tfunc(::JLTypeLattice, x, y) = widenconst(x)
229229

230+
function not_tfunc(𝕃::AbstractLattice, @nospecialize(b))
231+
if isa(b, Conditional)
232+
return Conditional(b.slot, b.elsetype, b.thentype)
233+
elseif isa(b, Const)
234+
return Const(not_int(b.val))
235+
end
236+
return math_tfunc(𝕃, b)
237+
end
238+
230239
add_tfunc(and_int, 2, 2, and_int_tfunc, 1)
231240
add_tfunc(or_int, 2, 2, or_int_tfunc, 1)
232241
add_tfunc(xor_int, 2, 2, math_tfunc, 1)
233-
add_tfunc(not_int, 1, 1, math_tfunc, 0) # usually used as not_int(::Bool) to negate a condition
242+
add_tfunc(not_int, 1, 1, not_tfunc, 0) # usually used as not_int(::Bool) to negate a condition
234243
add_tfunc(shl_int, 2, 2, shift_tfunc, 1)
235244
add_tfunc(lshr_int, 2, 2, shift_tfunc, 1)
236245
add_tfunc(ashr_int, 2, 2, shift_tfunc, 1)

0 commit comments

Comments
 (0)