Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lowering: Canonicalize to builtins for global assignment #56713

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4020,13 +4020,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState, nextr
end
effects === nothing || merge_override_effects!(interp, effects, frame)
if lhs !== nothing && rt !== Bottom
if isa(lhs, SlotNumber)
changes = StateUpdate(lhs, VarState(rt, false))
elseif isa(lhs, GlobalRef)
handle_global_assignment!(interp, frame, currsaw_latestworld, lhs, rt)
else
merge_effects!(interp, frame, EFFECTS_UNKNOWN)
end
changes = StateUpdate(lhs::SlotNumber, VarState(rt, false))
end
end
if !has_curr_ssaflag(frame, IR_FLAG_NOTHROW)
Expand Down
11 changes: 1 addition & 10 deletions Compiler/src/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1408,16 +1408,7 @@ function statement_cost(ex::Expr, line::Int, src::Union{CodeInfo, IRCode}, sptyp
extyp = line == -1 ? Any : argextype(SSAValue(line), src, sptypes)
return extyp === Union{} ? 0 : UNKNOWN_CALL_COST
elseif head === :(=)
if ex.args[1] isa GlobalRef
cost = UNKNOWN_CALL_COST
else
cost = 0
end
a = ex.args[2]
if a isa Expr
cost = plus_saturate(cost, statement_cost(a, -1, src, sptypes, params))
end
return cost
return statement_cost(ex.args[2], -1, src, sptypes, params)
elseif head === :copyast
return 100
end
Expand Down
12 changes: 0 additions & 12 deletions Compiler/src/ssair/EscapeAnalysis.jl
Original file line number Diff line number Diff line change
Expand Up @@ -642,13 +642,6 @@ function analyze_escapes(ir::IRCode, nargs::Int, 𝕃ₒ::AbstractLattice, get_e
escape_invoke!(astate, pc, stmt.args)
elseif head === :new || head === :splatnew
escape_new!(astate, pc, stmt.args)
elseif head === :(=)
lhs, rhs = stmt.args
if isa(lhs, GlobalRef) # global store
add_escape_change!(astate, rhs, ⊤)
else
unexpected_assignment!(ir, pc)
end
elseif head === :foreigncall
escape_foreigncall!(astate, pc, stmt.args)
elseif head === :throw_undef_if_not # XXX when is this expression inserted ?
Expand Down Expand Up @@ -981,11 +974,6 @@ function escape_unanalyzable_obj!(astate::AnalysisState, @nospecialize(obj), obj
return objinfo
end

@noinline function unexpected_assignment!(ir::IRCode, pc::Int)
@eval Main (ir = $ir; pc = $pc)
error("unexpected assignment found: inspect `Main.pc` and `Main.pc`")
end

is_nothrow(ir::IRCode, pc::Int) = has_flag(ir[SSAValue(pc)], IR_FLAG_NOTHROW)

# NOTE if we don't maintain the alias set that is separated from the lattice state, we can do
Expand Down
10 changes: 2 additions & 8 deletions Compiler/src/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,8 @@ function verify_ir(ir::IRCode, print::Bool=true,
isforeigncall = false
if isa(stmt, Expr)
if stmt.head === :(=)
if stmt.args[1] isa SSAValue
@verify_error "SSAValue as assignment LHS"
raise_error()
end
if stmt.args[2] isa GlobalRef
# undefined GlobalRef as assignment RHS is OK
continue
end
@verify_error "Assignment should have been removed during SSA conversion"
raise_error()
elseif stmt.head === :isdefined
if length(stmt.args) > 2 || (length(stmt.args) == 2 && !isa(stmt.args[2], Bool))
@verify_error "malformed isdefined"
Expand Down
2 changes: 1 addition & 1 deletion Compiler/test/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2111,7 +2111,7 @@ for run_finalizer_escape_test in (run_finalizer_escape_test1, run_finalizer_esca
global finalizer_escape::Int = 0

let src = code_typed1(run_finalizer_escape_test, Tuple{Bool, Bool})
@test any(x->isexpr(x, :(=)), src.code)
@test any(iscall((src, Core.setglobal!)), src.code)
end

let
Expand Down
24 changes: 5 additions & 19 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,25 +569,11 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
s->locals[n - 1] = rhs;
}
else {
jl_module_t *modu;
jl_sym_t *sym;
// Plain assignment is allowed to create bindings at
// toplevel and only for the current module
int alloc = toplevel;
if (jl_is_globalref(lhs)) {
modu = jl_globalref_mod(lhs);
sym = jl_globalref_name(lhs);
alloc &= modu == s->module;
}
else {
assert(jl_is_symbol(lhs));
modu = s->module;
sym = (jl_sym_t*)lhs;
}
JL_GC_PUSH1(&rhs);
jl_binding_t *b = jl_get_binding_wr(modu, sym, alloc);
jl_checked_assignment(b, modu, sym, rhs);
JL_GC_POP();
// This is an unmodeled error. Our frontend only generates
// legal `=` expressions, but since GlobalRef used to be legal
// here, give a loud error in case any package is modifying
// internals.
jl_error("Invalid IR: Assignment LHS not a Slot");
}
}
else if (head == jl_leave_sym) {
Expand Down
14 changes: 10 additions & 4 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -4607,14 +4607,20 @@ f(x) = yt(x)
(cdr cnd)
(list cnd))))))
tests))
(define (emit-assignment-or-setglobal lhs rhs)
(if (globalref? lhs)
(begin
(emit `(global ,lhs))
(emit `(call (top setglobal!) ,(cadr lhs) (inert ,(caddr lhs)) ,rhs)))
(emit `(= ,lhs ,rhs))))
(define (emit-assignment lhs rhs)
(if rhs
(if (valid-ir-rvalue? lhs rhs)
(emit `(= ,lhs ,rhs))
(emit-assignment-or-setglobal lhs rhs)
(let ((rr (make-ssavalue)))
(emit `(= ,rr ,rhs))
(emit `(= ,lhs ,rr))))
(emit `(= ,lhs (null)))) ; in unreachable code (such as after return), still emit the assignment so that the structure of those uses is preserved
(emit-assignment-or-setglobal lhs rr)))
(emit-assignment-or-setglobal lhs `(null))) ; in unreachable code (such as after return), still emit the assignment so that the structure of those uses is preserved
#f)
;; the interpreter loop. `break-labels` keeps track of the labels to jump to
;; for all currently closing break-blocks.
Expand Down Expand Up @@ -4693,7 +4699,7 @@ f(x) = yt(x)
rhs (make-ssavalue))))
(if (not (eq? rr rhs))
(emit `(= ,rr ,rhs)))
(emit `(= ,lhs ,rr))
(emit-assignment-or-setglobal lhs rr)
(if tail (emit-return tail rr))
rr)
(emit-assignment lhs rhs))))))
Expand Down
5 changes: 3 additions & 2 deletions test/syntax.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3713,7 +3713,7 @@ end
module Foreign54607
# Syntactic, not dynamic
try_to_create_binding1() = (Foreign54607.foo = 2)
# GlobalRef is allowed for same-module assignment
# GlobalRef is allowed for same-module assignment and declares the binding
@eval try_to_create_binding2() = ($(GlobalRef(Foreign54607, :foo2)) = 2)
function global_create_binding()
global bar
Expand All @@ -3728,7 +3728,7 @@ module Foreign54607
end
@test_throws ErrorException (Foreign54607.foo = 1)
@test_throws ErrorException Foreign54607.try_to_create_binding1()
@test_throws ErrorException Foreign54607.try_to_create_binding2()
Foreign54607.try_to_create_binding2()
function assign_in_foreign_module()
(Foreign54607.foo = 1)
nothing
Expand All @@ -3744,6 +3744,7 @@ Foreign54607.global_create_binding()
@test isdefined(Foreign54607, :baz)
@test isdefined(Foreign54607, :compiled_assign)
@test isdefined(Foreign54607, :gr_assign)
@test isdefined(Foreign54607, :foo2)
Foreign54607.bar = 8
@test Foreign54607.bar == 8
begin
Expand Down
Loading