lowering: Canonicalize to builtins for global assignment

This adjusts lowering to emit `setglobal!` for assignment to globals,
thus making the `=` expr head used only for slots in `CodeInfo` and
entirely absent in `IRCode`. The primary reason for this is just
to reduce the number of special cases that compiler passes have
to reason about. In IRCode, `=` was already essentially equivalent
to `setglobal!`, so there's no good reason not to canonicalize.

Finally, the `=` syntax form for globals already gets recognized
specially to insert `convert` calls to their declared binding
type, so this doesn't impose any additional requirements on
lowering to distinguish local from global assignments. In general,
I'd also like to separate syntax and intermediate forms as much
as possible where their semantics differ, which this accomplises
by just using the builtin.

This change is mostly semantically invisible, except that spliced-in
GlobalRefs now declare their binding because they are indistinguishable
from ordinary assignments at the stage where I inserted the lowering.
If we want to, we can preserve the difference, but it'd be a bit
more annoying for not much benefit, because:
1. The spliced in version was only recently made to work anyway, and
2. The semantics of when exactly bindings are declared is still messy
   on master anyway and will get tweaked shortly in further binding
   partitions work.

Author: Keno

Compiler/src/abstractinterpretation.jl

@@ -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)

Compiler/src/optimize.jl

@@ -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

Compiler/src/ssair/EscapeAnalysis.jl

@@ -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 ?
@@ -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

Compiler/src/ssair/verify.jl

@@ -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"

Compiler/test/inline.jl

@@ -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

src/interpreter.c

@@ -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) {

src/julia-syntax.scm

@@ -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.
@@ -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))))))

test/syntax.jl

@@ -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
@@ -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
@@ -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