Skip to content

Commit

Permalink
lowering: Refactor lowering for const and typed globals
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.
  • Loading branch information
Keno committed Jun 19, 2024
1 parent 6f39acb commit f00f2e6
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 @@ -2542,18 +2542,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 @@ -932,6 +932,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 @@ -6461,7 +6479,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 @@ -6471,10 +6489,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 f00f2e6

Please sign in to comment.