Skip to content

Commit 184eaf7

Browse files
authored
bpart: Start enforcing minimum world age for const bparts (JuliaLang#57102)
Currently, even though the binding partition system is implemented, it is largely enabled. New `const` definitions get magically "backdated" to the first world age in which the binding was undefined. Additionally, they do not get their own world age and there is currently no `latestworld` marker after `const` definitions. This PR changes this situation to give const markers their own world age with appropriate `latestworld` increments. Both of these are mandatory for `const` replacement to work. The other thing this PR does is prepare to remove the automatic "backdating". To see the difference, consider: ``` function foo($i) Core.eval(:(const x = $i)) x end ``` Without an intervening world age increment, this will throw an UndefVarError on this PR. I believe this is the best option for two reasons: 1. It will allow us infer these to `Union{}` in the future (thus letting inference prune dead code faster). 2. I think it is less confusing in terms of the world age semantics for `const` definitions to become active only after they are defined. To illustrate the second point, suppose we did keep the automatic backdating. Then we would have: ``` foo(1) => 1 foo(2) => 1 foo(3) => 2 ``` as opposed to on this PR: ``` foo(1) => UndefVarError foo(2) => 1 foo(3) => 2 ``` The semantics are consistent, of course, but I am concerned that an automatic backdating will give users the wrong mental model about world age, since it "fixes itself" the first time, but if you Revise it, it will give an unexpected answer. I think this would encourage accidentally bad code patterns that only break under Revise (where they are hard to debug). The counterpoint of course is that that not backdating is a more breaking choice. As with the rest of the 1.12-era world age semantics changes, I think taking a look at PkgEval will be helpful.
1 parent e500327 commit 184eaf7

File tree

7 files changed

+22
-15
lines changed

7 files changed

+22
-15
lines changed

src/abstractinterpretation.jl

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3456,10 +3456,10 @@ world_range(ir::IRCode) = ir.valid_worlds
34563456
world_range(ci::CodeInfo) = WorldRange(ci.min_world, ci.max_world)
34573457
world_range(compact::IncrementalCompact) = world_range(compact.ir)
34583458

3459-
function force_binding_resolution!(g::GlobalRef)
3459+
function force_binding_resolution!(g::GlobalRef, world::UInt)
34603460
# Force resolution of the binding
34613461
# TODO: This will go away once we switch over to fully partitioned semantics
3462-
ccall(:jl_globalref_boundp, Cint, (Any,), g)
3462+
ccall(:jl_force_binding_resolution, Cvoid, (Any, Csize_t), g, world)
34633463
return nothing
34643464
end
34653465

@@ -3477,7 +3477,7 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
34773477
# This method is surprisingly hot. For performance, don't ask the runtime to resolve
34783478
# the binding unless necessary - doing so triggers an additional lookup, which though
34793479
# not super expensive is hot enough to show up in benchmarks.
3480-
force_binding_resolution!(g)
3480+
force_binding_resolution!(g, min_world(worlds))
34813481
return abstract_eval_globalref_type(g, src, false)
34823482
end
34833483
# return Union{}
@@ -3490,7 +3490,7 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
34903490
end
34913491

34923492
function lookup_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
3493-
force_binding_resolution!(g)
3493+
force_binding_resolution!(g, get_inference_world(interp))
34943494
partition = lookup_binding_partition(get_inference_world(interp), g)
34953495
update_valid_age!(sv, WorldRange(partition.min_world, partition.max_world))
34963496
partition
@@ -3539,7 +3539,8 @@ function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, saw_
35393539
partition = abstract_eval_binding_partition!(interp, g, sv)
35403540
ret = abstract_eval_partition_load(interp, partition)
35413541
if ret.rt !== Union{} && ret.exct === UndefVarError && InferenceParams(interp).assume_bindings_static
3542-
if isdefined(g, :binding) && isdefined(g.binding, :value)
3542+
b = convert(Core.Binding, g)
3543+
if isdefined(b, :value)
35433544
ret = RTEffects(ret.rt, Union{}, Effects(generic_getglobal_effects, nothrow=true))
35443545
end
35453546
# We do not assume in general that assigned global bindings remain assigned.

src/optimize.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
12861286
# types of call arguments only once `slot2reg` converts this `IRCode` to the SSA form
12871287
# and eliminates slots (see below)
12881288
argtypes = sv.slottypes
1289-
return IRCode(stmts, sv.cfg, di, argtypes, meta, sv.sptypes, WorldRange(ci.min_world, ci.max_world))
1289+
return IRCode(stmts, sv.cfg, di, argtypes, meta, sv.sptypes, world_range(ci))
12901290
end
12911291

12921292
function process_meta!(meta::Vector{Expr}, @nospecialize stmt)

src/ssair/ir.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ struct IRCode
434434
function IRCode(stmts::InstructionStream, cfg::CFG, debuginfo::DebugInfoStream,
435435
argtypes::Vector{Any}, meta::Vector{Expr}, sptypes::Vector{VarState},
436436
valid_worlds=WorldRange(typemin(UInt), typemax(UInt)))
437-
return new(stmts, argtypes, sptypes, debuginfo, cfg, NewNodeStream(), meta)
437+
return new(stmts, argtypes, sptypes, debuginfo, cfg, NewNodeStream(), meta, valid_worlds)
438438
end
439439
function IRCode(ir::IRCode, stmts::InstructionStream, cfg::CFG, new_nodes::NewNodeStream)
440440
di = ir.debuginfo
@@ -1462,7 +1462,7 @@ function process_node!(compact::IncrementalCompact, result_idx::Int, inst::Instr
14621462
result[result_idx][:stmt] = GotoNode(label)
14631463
result_idx += 1
14641464
elseif isa(stmt, GlobalRef)
1465-
total_flags = IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE
1465+
total_flags = IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW
14661466
flag = result[result_idx][:flag]
14671467
if has_flag(flag, total_flags)
14681468
ssa_rename[idx] = stmt

src/ssair/legacy.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ function inflate_ir!(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{A
4444
di = DebugInfoStream(nothing, ci.debuginfo, nstmts)
4545
stmts = InstructionStream(code, ssavaluetypes, info, di.codelocs, ci.ssaflags)
4646
meta = Expr[]
47-
return IRCode(stmts, cfg, di, argtypes, meta, sptypes, WorldRange(ci.min_world, ci.max_world))
47+
return IRCode(stmts, cfg, di, argtypes, meta, sptypes, world_range(ci))
4848
end
4949

5050
"""

src/ssair/verify.jl

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ end
1212

1313
if !isdefined(@__MODULE__, Symbol("@verify_error"))
1414
macro verify_error(arg)
15-
arg isa String && return esc(:(print && println(stderr, $arg)))
15+
arg isa String && return esc(:(print && println($(GlobalRef(Core, :stderr)), $arg)))
1616
isexpr(arg, :string) || error("verify_error macro expected a string expression")
1717
pushfirst!(arg.args, GlobalRef(Core, :stderr))
1818
pushfirst!(arg.args, :println)
@@ -61,8 +61,14 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
6161
raise_error()
6262
end
6363
elseif isa(op, GlobalRef)
64-
if !isdefined(op.mod, op.name) || !isconst(op.mod, op.name)
65-
@verify_error "Unbound GlobalRef not allowed in value position"
64+
force_binding_resolution!(op, min_world(ir.valid_worlds))
65+
bpart = lookup_binding_partition(min_world(ir.valid_worlds), op)
66+
while is_some_imported(binding_kind(bpart)) && max_world(ir.valid_worlds) <= bpart.max_world
67+
imported_binding = partition_restriction(bpart)::Core.Binding
68+
bpart = lookup_binding_partition(min_world(ir.valid_worlds), imported_binding)
69+
end
70+
if !is_defined_const_binding(binding_kind(bpart)) || (bpart.max_world < max_world(ir.valid_worlds))
71+
@verify_error "Unbound or partitioned GlobalRef not allowed in value position"
6672
raise_error()
6773
end
6874
elseif isa(op, Expr)

src/tfuncs.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1109,7 +1109,7 @@ function _getfield_tfunc_const(@nospecialize(sv), name::Const)
11091109
if isa(sv, DataType) && nv == DATATYPE_TYPES_FIELDINDEX && isdefined(sv, nv)
11101110
return Const(getfield(sv, nv))
11111111
end
1112-
if isconst(typeof(sv), nv)
1112+
if !isa(sv, Module) && isconst(typeof(sv), nv)
11131113
if isdefined(sv, nv)
11141114
return Const(getfield(sv, nv))
11151115
end

test/inline.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ end
149149

150150
(src, _) = only(code_typed(sum27403, Tuple{Vector{Int}}))
151151
@test !any(src.code) do x
152-
x isa Expr && x.head === :invoke && x.args[2] !== Core.GlobalRef(Base, :throw_boundserror)
152+
x isa Expr && x.head === :invoke && !(x.args[2] in (Core.GlobalRef(Base, :throw_boundserror), Base.throw_boundserror))
153153
end
154154
end
155155

@@ -313,7 +313,7 @@ end
313313
const _a_global_array = [1]
314314
f_inline_global_getindex() = _a_global_array[1]
315315
let ci = code_typed(f_inline_global_getindex, Tuple{})[1].first
316-
@test any(x->(isexpr(x, :call) && x.args[1] === GlobalRef(Base, :memoryrefget)), ci.code)
316+
@test any(x->(isexpr(x, :call) && x.args[1] in (GlobalRef(Base, :memoryrefget), Base.memoryrefget)), ci.code)
317317
end
318318

319319
# Issue #29114 & #36087 - Inlining of non-tuple splats

0 commit comments

Comments
 (0)