Skip to content

Commit

Permalink
lowering: Refactor lowering for const and typed globals (#54773)
Browse files Browse the repository at this point in the history
This is a prepratory commit for #54654 to change the lowering of `const`
and typed globals to be compatible with the new semantics.

Currently, we lower `const a::T = val` to:
```
const a
global a::T
a = val
```
(which further expands to typed-globals an implicit converts).

This works, because, under the hood, our const declarations are actually
assign-once globals. Note however, that this is not syntactically
reachable, since we have a parse error for plain `const a`:

```
julia> const a
ERROR: ParseError:
# Error @ REPL[1]:1:1
const a
└─────┘ ── expected assignment after `const`
Stacktrace:
 [1] top-level scope
   @ none:1
```

However, this lowering is not atomic with respect to world age. The
semantics in #54654 require that the const-ness and the value are
established atomically (with respect to world age, potentially on
another thread) or undergo invalidation.

To resolve this issue, this PR changes the lowering of `const a::T =
val` to:
```
let
    local a::T = val
    const (global a) = a
end
```
where the latter is a special syntax form `Expr(:const, GlobalRef(,:a),
:a)`.

A similar change is made to const global declarations, which previously
lowered via intrinsic, i.e. `global a::T = val` lowered to:
```
global a
Core.set_binding_type!(Main, :a, T)
_T = Core.get_binding_type(Main, :a)
if !isa(val, _T)
    val = convert(_T, val)
end
a = val
```

This changes the `set_binding_type!` to instead be a syntax form
`Expr(:globaldecl, :a, T)`. This is not technically required, but we
currently do not use intrinsics for world-age affecting side-effects
anywhere else in the system. In particular, after #54654, it would be
illegal to call `set_binding_type!` in anything but top-level context.
Now, we have discussed in the past that there should potentially be
intrinsic functions for global modifications (method table additions,
etc), currently only reachable through `Core.eval`, but such an
intrinsic would require semantics that differ from both the current
`set_binding_type!` and the new `:globaldecl`. Using an Expr form here
is the most consistent with our current practice for these sort of
things elsewhere and accordingly, this PR removes the intrinsic.

Note that this PR does not yet change any syntax semantics, although
there could in principle be a reordering of side-effects within an
expression (e.g. things like `global a::(@isdefined(a) ? Int : Float64)`
might behave differently after this commit. However, we never defined
the order of side effects (which is part of what this is cleaning up,
although, I am not formally defining any specific ordering here either -
#54654 will do some of that), and that is not a common case, so this PR
should be largely considered non-semantic with respect to the syntax
change.

Also fixes #54787 while we're at it.
  • Loading branch information
Keno authored Jun 23, 2024
1 parent 2d4ef8a commit dfd1d49
Show file tree
Hide file tree
Showing 15 changed files with 360 additions and 169 deletions.
12 changes: 0 additions & 12 deletions base/docs/basedocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2570,18 +2570,6 @@ Retrieve the declared type of the binding `name` from the module `module`.
"""
Core.get_binding_type

"""
Core.set_binding_type!(module::Module, name::Symbol, [type::Type])
Set the declared type of the binding `name` in the module `module` to `type`. Error if the
binding already has a type that is not equivalent to `type`. If the `type` argument is
absent, set the binding type to `Any` if unset, but do not error.
!!! compat "Julia 1.9"
This function requires Julia 1.9 or later.
"""
Core.set_binding_type!

"""
swapglobal!(module::Module, name::Symbol, x, [order::Symbol=:monotonic])
Expand Down
1 change: 0 additions & 1 deletion doc/src/devdocs/builtins.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Core.Intrinsics.atomic_pointerswap
Core.Intrinsics.atomic_pointermodify
Core.Intrinsics.atomic_pointerreplace
Core.get_binding_type
Core.set_binding_type!
Core.IntrinsicFunction
Core.Intrinsics
Core.IR
Expand Down
2 changes: 2 additions & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ JL_DLLEXPORT jl_sym_t *jl_thunk_sym;
JL_DLLEXPORT jl_sym_t *jl_foreigncall_sym;
JL_DLLEXPORT jl_sym_t *jl_as_sym;
JL_DLLEXPORT jl_sym_t *jl_global_sym;
JL_DLLEXPORT jl_sym_t *jl_globaldecl_sym;
JL_DLLEXPORT jl_sym_t *jl_local_sym;
JL_DLLEXPORT jl_sym_t *jl_list_sym;
JL_DLLEXPORT jl_sym_t *jl_dot_sym;
Expand Down Expand Up @@ -355,6 +356,7 @@ void jl_init_common_symbols(void)
jl_opaque_closure_method_sym = jl_symbol("opaque_closure_method");
jl_const_sym = jl_symbol("const");
jl_global_sym = jl_symbol("global");
jl_globaldecl_sym = jl_symbol("globaldecl");
jl_local_sym = jl_symbol("local");
jl_thunk_sym = jl_symbol("thunk");
jl_toplevel_sym = jl_symbol("toplevel");
Expand Down
1 change: 0 additions & 1 deletion src/builtin_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ JL_CALLABLE(jl_f__primitivetype);
JL_CALLABLE(jl_f__setsuper);
JL_CALLABLE(jl_f__equiv_typedef);
JL_CALLABLE(jl_f_get_binding_type);
JL_CALLABLE(jl_f_set_binding_type);
JL_CALLABLE(jl_f__compute_sparams);
JL_CALLABLE(jl_f__svec_ref);
#ifdef __cplusplus
Expand Down
22 changes: 0 additions & 22 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1384,27 +1384,6 @@ JL_CALLABLE(jl_f_get_binding_type)
return ty;
}

JL_CALLABLE(jl_f_set_binding_type)
{
JL_NARGS(set_binding_type!, 2, 3);
jl_module_t *m = (jl_module_t*)args[0];
jl_sym_t *s = (jl_sym_t*)args[1];
JL_TYPECHK(set_binding_type!, module, (jl_value_t*)m);
JL_TYPECHK(set_binding_type!, symbol, (jl_value_t*)s);
jl_value_t *ty = nargs == 2 ? (jl_value_t*)jl_any_type : args[2];
JL_TYPECHK(set_binding_type!, type, ty);
jl_binding_t *b = jl_get_binding_wr(m, s, 0);
jl_value_t *old_ty = NULL;
if (jl_atomic_cmpswap_relaxed(&b->ty, &old_ty, ty)) {
jl_gc_wb(b, ty);
}
else if (nargs != 2 && !jl_types_equal(ty, old_ty)) {
jl_errorf("cannot set type for global %s.%s. It already has a value or is already set to a different type.",
jl_symbol_name(m->name), jl_symbol_name(s));
}
return jl_nothing;
}

JL_CALLABLE(jl_f_swapglobal)
{
enum jl_memory_order order = jl_memory_order_release;
Expand Down Expand Up @@ -2416,7 +2395,6 @@ void jl_init_primitives(void) JL_GC_DISABLED
jl_builtin_getglobal = add_builtin_func("getglobal", jl_f_getglobal);
jl_builtin_setglobal = add_builtin_func("setglobal!", jl_f_setglobal);
add_builtin_func("get_binding_type", jl_f_get_binding_type);
add_builtin_func("set_binding_type!", jl_f_set_binding_type);
jl_builtin_swapglobal = add_builtin_func("swapglobal!", jl_f_swapglobal);
jl_builtin_replaceglobal = add_builtin_func("replaceglobal!", jl_f_replaceglobal);
jl_builtin_modifyglobal = add_builtin_func("modifyglobal!", jl_f_modifyglobal);
Expand Down
45 changes: 41 additions & 4 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,24 @@ static const auto jldeclareconst_func = new JuliaFunction<>{
{T_pjlvalue, T_pjlvalue, T_pjlvalue}, false); },
nullptr,
};
static const auto jldeclareconstval_func = new JuliaFunction<>{
XSTR(jl_declare_constant_val),
[](LLVMContext &C) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
auto T_prjlvalue = JuliaType::get_prjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, T_pjlvalue, T_pjlvalue, T_prjlvalue}, false); },
nullptr,
};
static const auto jldeclareglobal_func = new JuliaFunction<>{
XSTR(jl_declare_global),
[](LLVMContext &C) {
auto T_pjlvalue = JuliaType::get_pjlvalue_ty(C);
auto T_prjlvalue = JuliaType::get_prjlvalue_ty(C);
return FunctionType::get(getVoidTy(C),
{T_pjlvalue, T_pjlvalue, T_prjlvalue}, false); },
nullptr,
};
static const auto jlgetbindingorerror_func = new JuliaFunction<>{
XSTR(jl_get_binding_or_error),
[](LLVMContext &C) {
Expand Down Expand Up @@ -6422,7 +6440,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
return meth;
}
else if (head == jl_const_sym) {
assert(nargs == 1);
assert(nargs <= 2);
jl_sym_t *sym = (jl_sym_t*)args[0];
jl_module_t *mod = ctx.module;
if (jl_is_globalref(sym)) {
Expand All @@ -6432,10 +6450,29 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
if (jl_is_symbol(sym)) {
jl_binding_t *bnd = NULL;
Value *bp = global_binding_pointer(ctx, mod, sym, &bnd, true, true);
if (bp)
ctx.builder.CreateCall(prepare_call(jldeclareconst_func),
{ bp, literal_pointer_val(ctx, (jl_value_t*)mod), literal_pointer_val(ctx, (jl_value_t*)sym) });
if (bp) {
if (nargs == 2) {
jl_cgval_t rhs = emit_expr(ctx, args[1]);
ctx.builder.CreateCall(prepare_call(jldeclareconstval_func),
{ bp, literal_pointer_val(ctx, (jl_value_t*)mod), literal_pointer_val(ctx, (jl_value_t*)sym), boxed(ctx, rhs) });
} else {
ctx.builder.CreateCall(prepare_call(jldeclareconst_func),
{ bp, literal_pointer_val(ctx, (jl_value_t*)mod), literal_pointer_val(ctx, (jl_value_t*)sym) });
}
}
}
}
else if (head == jl_globaldecl_sym) {
assert(nargs == 2);
jl_sym_t *sym = (jl_sym_t*)args[0];
jl_module_t *mod = ctx.module;
if (jl_is_globalref(sym)) {
mod = jl_globalref_mod(sym);
sym = jl_globalref_name(sym);
}
jl_cgval_t typ = emit_expr(ctx, args[1]);
ctx.builder.CreateCall(prepare_call(jldeclareglobal_func),
{ literal_pointer_val(ctx, (jl_value_t*)mod), literal_pointer_val(ctx, (jl_value_t*)sym), boxed(ctx, typ) });
}
else if (head == jl_new_sym) {
bool is_promotable = false;
Expand Down
12 changes: 12 additions & 0 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,18 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
jl_value_t *res = jl_toplevel_eval(s->module, stmt);
s->locals[jl_source_nslots(s->src) + s->ip] = res;
}
else if (head == jl_globaldecl_sym) {
jl_value_t *val = eval_value(jl_exprarg(stmt, 1), s);
s->locals[jl_source_nslots(s->src) + s->ip] = val; // temporarily root
jl_declare_global(s->module, jl_exprarg(stmt, 0), val);
s->locals[jl_source_nslots(s->src) + s->ip] = jl_nothing;
}
else if (head == jl_const_sym) {
jl_value_t *val = jl_expr_nargs(stmt) == 1 ? NULL : eval_value(jl_exprarg(stmt, 1), s);
s->locals[jl_source_nslots(s->src) + s->ip] = val; // temporarily root
jl_eval_const_decl(s->module, jl_exprarg(stmt, 0), val);
s->locals[jl_source_nslots(s->src) + s->ip] = jl_nothing;
}
else if (jl_is_toplevel_only_expr(stmt)) {
jl_toplevel_eval(s->module, stmt);
}
Expand Down
Loading

0 comments on commit dfd1d49

Please sign in to comment.