From 1001750cb8e31bc28be151795332c1cd78405abe Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliahub.com>
Date: Fri, 29 Nov 2024 07:34:25 +0000
Subject: [PATCH 1/3] Fix scope of hoisted signature-local variables

When we declare inner methods, e.g. the `f` in

```
function fs()
   f(lhs::Integer) = 1
   f(lhs::Integer, rhs::(local x=Integer; x)) = 2
   return f
end
```

we must hoist the definition of the (appropriately mangled) generic
function `f` to top-level, including all variables that were used
in the signature definition of `f`. This situation is a bit
unique in the language because it uses inner function scope, but gets
executed in toplevel scope. For example, you're not allowed to use a local
of the inner function in the signature definition:

```
julia> function fs()
          local x=Integer
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[3]:1
```

In particular, the restriction is signature-local:
```
julia> function fs()
          f(rhs::(local x=Integer; x)) = 1
          f(lhs::Integer, rhs::x) = 2
          return f
       end
ERROR: syntax: local variable x cannot be used in closure declaration
Stacktrace:
 [1] top-level scope
   @ REPL[4]:1
```

There's a special intermediate form `moved-local` that gets generated
for this definition. In c6c3d72d1cbddb3d27e0df0e739bb27dd709a413, this form
stopped getting generated for certain inner methods. I suspect this
happened because of the incorrect assumption that the set of moved
locals is being computed over all signatures, rather than being a
per-signature property.

The result of all of this was that this is one of the few places
where lowering still generated a symbol as the lhs of an assignment
for a global (instead of globalref), because the code that generates
the assignment assumes it's a local, but the later pass doesn't know this.
Because we still retain the code for this from before we started using globalref
consistently, this wasn't generally causing a problems, except possibly leaking
a global (or potentially assigning to a global when this wasn't intended).
However, in follow on work, I want to make use of knowing whether the LHS is a
global or local in lowering, so this was causing me trouble.

Fix all of this by putting back the `moved-local` where it was dropped.

Fixes #56711
---
 src/julia-syntax.scm | 1 +
 test/syntax.jl       | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index 72e97da3c2daa..852d5eb4d6f86 100644
--- a/src/julia-syntax.scm
+++ b/src/julia-syntax.scm
@@ -4279,6 +4279,7 @@ f(x) = yt(x)
                    (if (or exists (and short (pair? alldefs)))
                        `(toplevel-butfirst
                          (null)
+                         ,@(map (lambda (v) `(moved-local ,v)) moved-vars)
                          ,@sp-inits
                          ,@mk-method
                          (latestworld))
diff --git a/test/syntax.jl b/test/syntax.jl
index d9d311ac6615d..0fb752bae480f 100644
--- a/test/syntax.jl
+++ b/test/syntax.jl
@@ -4033,3 +4033,11 @@ end
 @test isa(create_inner_f_no_methods(), Function)
 @test length(methods(create_inner_f_no_methods())) == 0
 @test Base.invoke_in_world(first(methods(create_inner_f_one_method)).primary_world, create_inner_f_one_method()) == 1
+
+# Issue 56711 - Scope of signature hoisting
+function fs56711()
+    f(lhs::Integer) = 1
+    f(lhs::Integer, rhs::(local x_should_not_be_defined=Integer; x_should_not_be_defined)) = 2
+    return f
+end
+@test !@isdefined(x_should_not_be_defined)

From 18205fb7f7974ab28d651d8e56c923aa8fdb4dc1 Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliahub.com>
Date: Fri, 29 Nov 2024 07:49:12 +0000
Subject: [PATCH 2/3] 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.
---
 Compiler/src/abstractinterpretation.jl |  8 +-------
 Compiler/src/optimize.jl               | 11 +----------
 Compiler/src/ssair/EscapeAnalysis.jl   | 12 ------------
 Compiler/src/ssair/verify.jl           | 10 ++--------
 Compiler/test/inline.jl                |  2 +-
 src/interpreter.c                      | 24 +++++-------------------
 src/julia-syntax.scm                   | 14 ++++++++++----
 test/syntax.jl                         |  5 +++--
 8 files changed, 23 insertions(+), 63 deletions(-)

diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl
index 5946adf80ad52..2590f6b7716bd 100644
--- a/Compiler/src/abstractinterpretation.jl
+++ b/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)
diff --git a/Compiler/src/optimize.jl b/Compiler/src/optimize.jl
index d2dfd26bfa00d..856e64e404388 100644
--- a/Compiler/src/optimize.jl
+++ b/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
diff --git a/Compiler/src/ssair/EscapeAnalysis.jl b/Compiler/src/ssair/EscapeAnalysis.jl
index 47a7840628bb5..4f32550d056b2 100644
--- a/Compiler/src/ssair/EscapeAnalysis.jl
+++ b/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
diff --git a/Compiler/src/ssair/verify.jl b/Compiler/src/ssair/verify.jl
index 59051058e1750..072a564a31f78 100644
--- a/Compiler/src/ssair/verify.jl
+++ b/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"
diff --git a/Compiler/test/inline.jl b/Compiler/test/inline.jl
index 46b78db3b781c..5f95fb761859e 100644
--- a/Compiler/test/inline.jl
+++ b/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
diff --git a/src/interpreter.c b/src/interpreter.c
index 49a3afed14f0c..d6c42834fbd36 100644
--- a/src/interpreter.c
+++ b/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) {
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index 852d5eb4d6f86..c7ca5d553bb31 100644
--- a/src/julia-syntax.scm
+++ b/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))))))
diff --git a/test/syntax.jl b/test/syntax.jl
index 0fb752bae480f..315f2d8b0f38b 100644
--- a/test/syntax.jl
+++ b/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

From c1869ec6cf5e2dd4b96a223850f6fac0c8ed735d Mon Sep 17 00:00:00 2001
From: Keno Fischer <keno@juliahub.com>
Date: Fri, 29 Nov 2024 08:01:17 +0000
Subject: [PATCH 3/3] lowering: Revert undefined global outlining

This partially reverts #51970, once again allowing throwing GlobalRefs
in value position for `CodeInfo`, (but not `IRCode`). The primary motivation
for this reversal is that after binding partition the throwiness of
a global is world-age dependent and we do not want to have lowering
be world-age dependent, because that would require us to rewrite
the lowered code upon invalidation (#56649).

With respect to the original motivation for this change (being able
to use the statement flag in inference), we retain the property
that the statement flags apply only to the optimizer version
of the statement (i.e. with the GlobalRefs moved out). The only
place in inference where we were not doing this, we can rely
on `exct` instead. This does assume that `exct` is precise, but
we've done a lot of work on that in the past year, so hopefully
we can rely on that now.

The revert is partial, because we go in the opposite direction in
a few places: For `GotoIfNot` cond and `EnterNode` scope operands, we
just forbid any `GlobalRef` entirely. Constants are rare in these,
so no need to burden inference with the possibility of handling
these. We do however, keep (and add appropriate handling code)
for GlobalRef arguments to `ReturnNode` to make sure that trivial
code remains one statement.
---
 Compiler/src/abstractinterpretation.jl |  98 ++++++++++++++--------
 Compiler/src/optimize.jl               | 107 +++++++++++++++----------
 Compiler/src/ssair/irinterp.jl         |   4 +-
 Compiler/src/ssair/verify.jl           |   2 +-
 src/ast.c                              |  37 ---------
 src/jlfrontend.scm                     |   1 -
 src/julia-syntax.scm                   |   6 +-
 test/core.jl                           |  33 ++++----
 8 files changed, 151 insertions(+), 137 deletions(-)

diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl
index 2590f6b7716bd..94abf1cdaedb5 100644
--- a/Compiler/src/abstractinterpretation.jl
+++ b/Compiler/src/abstractinterpretation.jl
@@ -2862,7 +2862,7 @@ function sp_type_rewrap(@nospecialize(T), mi::MethodInstance, isreturn::Bool)
 end
 
 function abstract_eval_cfunction(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState)
-    f = abstract_eval_value(interp, e.args[2], sstate, sv)
+    (f, exct) = abstract_eval_value(interp, e.args[2], sstate, sv)
     # rt = sp_type_rewrap(e.args[3], sv.linfo, true)
     atv = e.args[4]::SimpleVector
     at = Vector{Any}(undef, length(atv) + 1)
@@ -2921,30 +2921,32 @@ function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, sv::AbsI
         # @assert false "Unexpected EXPR head in value position"
         merge_effects!(interp, sv, EFFECTS_UNKNOWN)
     end
-    return Any
+    return Pair{Any, Any}(Any, Any)
 end
 
 function abstract_eval_value(interp::AbstractInterpreter, @nospecialize(e), sstate::StatementState, sv::AbsIntState)
     if isa(e, Expr)
         return abstract_eval_value_expr(interp, e, sv)
     else
-        (;rt, effects) = abstract_eval_special_value(interp, e, sstate, sv)
+        (;rt, exct, effects) = abstract_eval_special_value(interp, e, sstate, sv)
         merge_effects!(interp, sv, effects)
-        return collect_limitations!(rt, sv)
+        return Pair{Any, Any}(collect_limitations!(rt, sv), exct)
     end
 end
 
 function collect_argtypes(interp::AbstractInterpreter, ea::Vector{Any}, sstate::StatementState, sv::AbsIntState)
     n = length(ea)
     argtypes = Vector{Any}(undef, n)
+    exct = Union{}
     @inbounds for i = 1:n
-        ai = abstract_eval_value(interp, ea[i], sstate, sv)
+        (ai, ei) = abstract_eval_value(interp, ea[i], sstate, sv)
         if ai === Bottom
             return nothing
         end
         argtypes[i] = ai
+        exct = Union{exct, ei}
     end
-    return argtypes
+    return Pair{Vector{Any}, Any}(argtypes, exct)
 end
 
 struct RTEffects
@@ -2984,21 +2986,23 @@ function abstract_eval_call(interp::AbstractInterpreter, e::Expr, sstate::Statem
     if argtypes === nothing
         return Future(RTEffects(Bottom, Any, Effects()))
     end
+    (argtypes, aexct) = argtypes
     arginfo = ArgInfo(ea, argtypes)
     call = abstract_call(interp, arginfo, sstate, sv)::Future
     return Future{RTEffects}(call, interp, sv) do call, interp, sv
         (; rt, exct, effects, refinements) = call
-        return RTEffects(rt, exct, effects, refinements)
+        return RTEffects(rt, tmerge(typeinf_lattice(interp), exct, aexct), effects, refinements)
     end
 end
 
-
+const generic_new_exct = Union{ErrorException, TypeError}
 function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::StatementState,
                            sv::AbsIntState)
     𝕃ᵢ = typeinf_lattice(interp)
-    rt, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], sstate, sv), true)
+    (ea1, a1exct) = abstract_eval_value(interp, e.args[1], sstate, sv)
+    rt, isexact = instanceof_tfunc(ea1, true)
     ut = unwrap_unionall(rt)
-    exct = Union{ErrorException,TypeError}
+    nothrow = false
     if isa(ut, DataType) && !isabstracttype(ut)
         ismutable = ismutabletype(ut)
         fcount = datatype_fieldcount(ut)
@@ -3018,12 +3022,15 @@ function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::Stateme
         end
         if isconcretedispatch(rt)
             nothrow = true
+            exct = Union{}
             @assert fcount !== nothing && fcount ≥ nargs "malformed :new expression" # syntactically enforced by the front-end
             ats = Vector{Any}(undef, nargs)
             local anyrefine = false
             local allconst = true
             for i = 1:nargs
-                at = widenslotwrapper(abstract_eval_value(interp, e.args[i+1], sstate, sv))
+                (atn, aexct) = abstract_eval_value(interp, e.args[i+1], sstate, sv)
+                exct = Union{exct, aexct}
+                at = widenslotwrapper(atn)
                 ft = fieldtype(rt, i)
                 nothrow && (nothrow = ⊑(𝕃ᵢ, at, ft))
                 at = tmeet(𝕃ᵢ, at, ft)
@@ -3039,6 +3046,7 @@ function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::Stateme
                 end
                 ats[i] = at
             end
+            nothrow || (exct = Union{exct, TypeError})
             if fcount == nargs && consistent === ALWAYS_TRUE && allconst
                 argvals = Vector{Any}(undef, nargs)
                 for j in 1:nargs
@@ -3054,13 +3062,13 @@ function abstract_eval_new(interp::AbstractInterpreter, e::Expr, sstate::Stateme
             end
         else
             rt = refine_partial_type(rt)
-            nothrow = false
+            exct = generic_new_exct
         end
     else
         consistent = ALWAYS_FALSE
-        nothrow = false
+        exct = generic_new_exct
     end
-    nothrow && (exct = Union{})
+    exct = Union{exct, a1exct}
     effects = Effects(EFFECTS_TOTAL; consistent, nothrow)
     return RTEffects(rt, exct, effects)
 end
@@ -3068,10 +3076,12 @@ end
 function abstract_eval_splatnew(interp::AbstractInterpreter, e::Expr, sstate::StatementState,
                                 sv::AbsIntState)
     𝕃ᵢ = typeinf_lattice(interp)
-    rt, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], sstate, sv), true)
+    (a1, a1exct) = abstract_eval_value(interp, e.args[1], sstate, sv)
+    rt, isexact = instanceof_tfunc(a1, true)
     nothrow = false
     if length(e.args) == 2 && isconcretedispatch(rt) && !ismutabletype(rt)
-        at = abstract_eval_value(interp, e.args[2], sstate, sv)
+        (at, a2exct) = abstract_eval_value(interp, e.args[2], sstate, sv)
+        exct = Union{a1exct, a2exct}
         n = fieldcount(rt)
         if (isa(at, Const) && isa(at.val, Tuple) && n == length(at.val::Tuple) &&
             (let t = rt, at = at
@@ -3087,12 +3097,14 @@ function abstract_eval_splatnew(interp::AbstractInterpreter, e::Expr, sstate::St
             nothrow = isexact
             rt = PartialStruct(𝕃ᵢ, rt, at.fields::Vector{Any})
         end
+        nothrow || (exct = Union{exct, generic_new_exct})
     else
         rt = refine_partial_type(rt)
+        exct = Any
     end
     consistent = !ismutabletype(rt) ? ALWAYS_TRUE : CONSISTENT_IF_NOTRETURNED
     effects = Effects(EFFECTS_TOTAL; consistent, nothrow)
-    return RTEffects(rt, Any, effects)
+    return RTEffects(rt, exct, effects)
 end
 
 function abstract_eval_new_opaque_closure(interp::AbstractInterpreter, e::Expr, sstate::StatementState,
@@ -3107,6 +3119,7 @@ function abstract_eval_new_opaque_closure(interp::AbstractInterpreter, e::Expr,
             rt = Bottom
             effects = EFFECTS_THROWS
         else
+            (argtypes, _) = argtypes
             mi = frame_instance(sv)
             rt = opaque_closure_tfunc(𝕃ᵢ, argtypes[1], argtypes[2], argtypes[3],
                 argtypes[5], argtypes[6:end], mi)
@@ -3134,7 +3147,7 @@ end
 function abstract_eval_copyast(interp::AbstractInterpreter, e::Expr, sstate::StatementState,
                                sv::AbsIntState)
     effects = EFFECTS_UNKNOWN
-    rt = abstract_eval_value(interp, e.args[1], sstate, sv)
+    (rt, _) = abstract_eval_value(interp, e.args[1], sstate, sv)
     if rt isa Const && rt.val isa Expr
         # `copyast` makes copies of Exprs
         rt = Expr
@@ -3190,7 +3203,7 @@ function abstract_eval_isdefined(interp::AbstractInterpreter, @nospecialize(sym)
 end
 
 function abstract_eval_throw_undef_if_not(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState)
-    condt = abstract_eval_value(interp, e.args[2], sstate, sv)
+    (condt, argexct) = abstract_eval_value(interp, e.args[2], sstate, sv)
     condval = maybe_extract_const_bool(condt)
     rt = Nothing
     exct = UndefVarError
@@ -3205,7 +3218,7 @@ function abstract_eval_throw_undef_if_not(interp::AbstractInterpreter, e::Expr,
     elseif !hasintersect(widenconst(condt), Bool)
         rt = Union{}
     end
-    return RTEffects(rt, exct, effects)
+    return RTEffects(rt, Union{exct, argexct}, effects)
 end
 
 function abstract_eval_the_exception(::AbstractInterpreter, sv::InferenceState)
@@ -3275,8 +3288,8 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, ssta
     # N.B.: abstract_eval_value_expr can modify the global effects, but
     # we move out any arguments with effects during SSA construction later
     # and recompute the effects.
-    rt = abstract_eval_value_expr(interp, e, sv)
-    return RTEffects(rt, Any, EFFECTS_TOTAL)
+    (rt, exct) = abstract_eval_value_expr(interp, e, sv)
+    return RTEffects(rt, exct, EFFECTS_TOTAL)
 end
 
 # refine the result of instantiation of partially-known type `t` if some invariant can be assumed
@@ -3296,12 +3309,14 @@ function abstract_eval_foreigncall(interp::AbstractInterpreter, e::Expr, sstate:
     mi = frame_instance(sv)
     t = sp_type_rewrap(e.args[2], mi, true)
     for i = 3:length(e.args)
-        if abstract_eval_value(interp, e.args[i], sstate, sv) === Bottom
+        (at, aexct) = abstract_eval_value(interp, e.args[i], sstate, sv)
+        if at === Bottom
             return RTEffects(Bottom, Any, EFFECTS_THROWS)
         end
     end
     effects = foreigncall_effects(e) do @nospecialize x
-        abstract_eval_value(interp, x, sstate, sv)
+        (at, aexct) = abstract_eval_value(interp, x, sstate, sv)
+        at
     end
     cconv = e.args[5]
     if isa(cconv, QuoteNode) && (v = cconv.value; isa(v, Tuple{Symbol, UInt16}))
@@ -3367,6 +3382,10 @@ end
 
 function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode, IncrementalCompact}, retry_after_resolve::Bool=true)
     worlds = world_range(src)
+    abstract_eval_globalref_type(g, worlds, retry_after_resolve)
+end
+
+function abstract_eval_globalref_type(g::GlobalRef, worlds::WorldRange, retry_after_resolve::Bool=true)
     partition = lookup_binding_partition(min_world(worlds), g)
     partition.max_world < max_world(worlds) && return Any
     while is_some_imported(binding_kind(partition))
@@ -3380,7 +3399,7 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
             # the binding unless necessary - doing so triggers an additional lookup, which though
             # not super expensive is hot enough to show up in benchmarks.
             force_binding_resolution!(g)
-            return abstract_eval_globalref_type(g, src, false)
+            return abstract_eval_globalref_type(g, worlds, false)
         end
         # return Union{}
         return Any
@@ -3391,6 +3410,11 @@ function abstract_eval_globalref_type(g::GlobalRef, src::Union{CodeInfo, IRCode,
     return partition_restriction(partition)
 end
 
+function is_global_nothrow_const_in_all_worlds(g::GlobalRef, worlds::WorldRange, retry_after_resolve::Bool=true)
+    # TODO: We may be fine if there's two different partitions with different constants
+    return isa(abstract_eval_globalref_type(g, worlds, retry_after_resolve), Const)
+end
+
 function abstract_eval_binding_partition!(interp::AbstractInterpreter, g::GlobalRef, sv::AbsIntState)
     force_binding_resolution!(g)
     partition = lookup_binding_partition(get_inference_world(interp), g)
@@ -3821,7 +3845,8 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState, nextr
                 elseif isa(stmt, GotoIfNot)
                     condx = stmt.cond
                     condslot = ssa_def_slot(condx, frame)
-                    condt = abstract_eval_value(interp, condx, StatementState(currstate, currsaw_latestworld), frame)
+                    condt, aexct = abstract_eval_value(interp, condx, StatementState(currstate, currsaw_latestworld), frame)
+                    @assert aexct === Bottom # Guaranteed in lowering
                     if condt === Bottom
                         ssavaluetypes[currpc] = Bottom
                         empty!(frame.pclimitations)
@@ -3908,7 +3933,11 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState, nextr
                         @goto fallthrough
                     end
                 elseif isa(stmt, ReturnNode)
-                    rt = abstract_eval_value(interp, stmt.val, StatementState(currstate, currsaw_latestworld), frame)
+                    rt, rexct = abstract_eval_value(interp, stmt.val, StatementState(currstate, currsaw_latestworld), frame)
+                    if rexct !== Union{}
+                        update_exc_bestguess!(interp, rexct, frame)
+                        propagate_to_error_handler!(currstate, currsaw_latestworld, frame, 𝕃ᵢ)
+                    end
                     if update_bestguess!(interp, frame, currstate, rt)
                         update_cycle_worklists!(frame) do caller::InferenceState, caller_pc::Int
                             # no reason to revisit if that call-site doesn't affect the final result
@@ -3921,7 +3950,8 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState, nextr
                     ssavaluetypes[currpc] = Any
                     add_curr_ssaflag!(frame, IR_FLAG_NOTHROW)
                     if isdefined(stmt, :scope)
-                        scopet = abstract_eval_value(interp, stmt.scope, StatementState(currstate, currsaw_latestworld), frame)
+                        scopet, sexct = abstract_eval_value(interp, stmt.scope, StatementState(currstate, currsaw_latestworld), frame)
+                        @assert sexct === Bottom # Guaranteed in lowering
                         handler = gethandler(frame, currpc + 1)::TryCatchFrame
                         @assert handler.scopet !== nothing
                         if !⊑(𝕃ᵢ, scopet, handler.scopet)
@@ -4023,13 +4053,13 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState, nextr
                     changes = StateUpdate(lhs::SlotNumber, VarState(rt, false))
                 end
             end
-            if !has_curr_ssaflag(frame, IR_FLAG_NOTHROW)
-                if exct !== Union{}
-                    update_exc_bestguess!(interp, exct, frame)
-                    # TODO: assert that these conditions match. For now, we assume the `nothrow` flag
-                    # to be correct, but allow the exct to be an over-approximation.
-                end
+            if exct !== Union{}
+                update_exc_bestguess!(interp, exct, frame)
                 propagate_to_error_handler!(currstate, currsaw_latestworld, frame, 𝕃ᵢ)
+                # TODO: assert that these conditions match. For now, we assume the `nothrow` flag
+                # to be correct, but allow the exct to be an over-approximation.
+            else
+                has_curr_ssaflag(frame, IR_FLAG_NOTHROW)
             end
             if rt === Bottom
                 ssavaluetypes[currpc] = Bottom
diff --git a/Compiler/src/optimize.jl b/Compiler/src/optimize.jl
index 856e64e404388..668cfb0e78061 100644
--- a/Compiler/src/optimize.jl
+++ b/Compiler/src/optimize.jl
@@ -163,12 +163,14 @@ mutable struct OptimizationState{Interp<:AbstractInterpreter}
     unreachable::BitSet
     bb_vartables::Vector{Union{Nothing,VarTable}}
     insert_coverage::Bool
+    valid_worlds::WorldRange
 end
 function OptimizationState(sv::InferenceState, interp::AbstractInterpreter)
     inlining = InliningState(sv, interp)
     return OptimizationState(sv.linfo, sv.src, nothing, sv.stmt_info, sv.mod,
                              sv.sptypes, sv.slottypes, inlining, sv.cfg,
-                             sv.unreachable, sv.bb_vartables, sv.insert_coverage)
+                             sv.unreachable, sv.bb_vartables, sv.insert_coverage,
+                             sv.world.valid_worlds)
 end
 function OptimizationState(mi::MethodInstance, src::CodeInfo, interp::AbstractInterpreter)
     # prepare src for running optimization passes if it isn't already
@@ -200,7 +202,8 @@ function OptimizationState(mi::MethodInstance, src::CodeInfo, interp::AbstractIn
             for slot = 1:nslots
         ])
     end
-    return OptimizationState(mi, src, nothing, stmt_info, mod, sptypes, slottypes, inlining, cfg, unreachable, bb_vartables, false)
+    return OptimizationState(mi, src, nothing, stmt_info, mod, sptypes, slottypes, inlining, cfg, unreachable, bb_vartables, false,
+        WorldRange(get_inference_world(interp), get_inference_world(interp)))
 end
 function OptimizationState(mi::MethodInstance, interp::AbstractInterpreter)
     world = get_inference_world(interp)
@@ -1175,31 +1178,52 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
     nstmts = length(code)
     ssachangemap = labelchangemap = blockchangemap = nothing
     prevloc = 0
+
+    function insert_statement_here!(@nospecialize(new_stmt), @nospecialize(new_typ); after=false)
+        insert_idx = after ? idx + 1 : idx
+        insert!(code, insert_idx, new_stmt)
+        splice!(codelocs, 3idx-2:3idx-3, (codelocs[3idx-2], codelocs[3idx-1], codelocs[3idx-0]))
+        insert!(ssavaluetypes, insert_idx, new_typ)
+        insert!(stmtinfo, insert_idx, NoCallInfo())
+        insert!(ssaflags, insert_idx, IR_FLAG_NULL)
+        if ssachangemap === nothing
+            ssachangemap = fill(0, nstmts)
+        end
+        if labelchangemap === nothing
+            labelchangemap = fill(0, nstmts)
+        end
+        ssachangemap[after ? oldidx + 1 : oldidx] += 1
+        if oldidx < length(labelchangemap)
+            labelchangemap[oldidx + 1] += 1
+        end
+        if blockchangemap === nothing
+            blockchangemap = fill(0, length(sv.cfg.blocks))
+        end
+        blockchangemap[block_for_inst(sv.cfg, oldidx)] += 1
+        idx += 1
+    end
+
     while idx <= length(code)
         if sv.insert_coverage && changed_lineinfo(ci.debuginfo, oldidx, prevloc)
             # insert a side-effect instruction before the current instruction in the same basic block
-            insert!(code, idx, Expr(:code_coverage_effect))
-            splice!(codelocs, 3idx-2:3idx-3, (codelocs[3idx-2], codelocs[3idx-1], codelocs[3idx-0]))
-            insert!(ssavaluetypes, idx, Nothing)
-            insert!(stmtinfo, idx, NoCallInfo())
-            insert!(ssaflags, idx, IR_FLAG_NULL)
-            if ssachangemap === nothing
-                ssachangemap = fill(0, nstmts)
-            end
-            if labelchangemap === nothing
-                labelchangemap = fill(0, nstmts)
-            end
-            ssachangemap[oldidx] += 1
-            if oldidx < length(labelchangemap)
-                labelchangemap[oldidx + 1] += 1
-            end
-            if blockchangemap === nothing
-                blockchangemap = fill(0, length(sv.cfg.blocks))
-            end
-            blockchangemap[block_for_inst(sv.cfg, oldidx)] += 1
-            idx += 1
+            insert_statement_here!(Expr(:code_coverage_effect), Nothing)
             prevloc = oldidx
         end
+        stmt = code[idx]
+        # Move out any GlobalRefs that may throw into statement position to satisfy IRCode invariants
+        updated_any = false
+        if !isa(stmt, GlobalRef) && !isexpr(stmt, :isdefined)
+            urs = userefs(stmt)
+            for ur in urs
+                arg = ur[]
+                if isa(arg, GlobalRef) && !is_global_nothrow_const_in_all_worlds(arg, sv.valid_worlds)
+                    ur[] = NewSSAValue(idx)
+                    insert_statement_here!(arg, abstract_eval_globalref_type(arg, sv.valid_worlds))
+                    updated_any = true
+                end
+            end
+            updated_any && (code[idx] = urs[])
+        end
         if ssavaluetypes[idx] === Union{} && !(oldidx in sv.unreachable) && !isa(code[idx], PhiNode)
             # We should have converted any must-throw terminators to an equivalent w/o control-flow edges
             @assert !isterminator(code[idx])
@@ -1233,26 +1257,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
                     ssaflags[block_end] = IR_FLAG_NOTHROW
                     idx += block_end - idx
                 else
-                    insert!(code, idx + 1, ReturnNode())
-                    splice!(codelocs, 3idx-2:3idx-3, (codelocs[3idx-2], codelocs[3idx-1], codelocs[3idx-0]))
-                    insert!(ssavaluetypes, idx + 1, Union{})
-                    insert!(stmtinfo, idx + 1, NoCallInfo())
-                    insert!(ssaflags, idx + 1, IR_FLAG_NOTHROW)
-                    if ssachangemap === nothing
-                        ssachangemap = fill(0, nstmts)
-                    end
-                    if labelchangemap === nothing
-                        labelchangemap = sv.insert_coverage ? fill(0, nstmts) : ssachangemap
-                    end
-                    if oldidx < length(ssachangemap)
-                        ssachangemap[oldidx + 1] += 1
-                        sv.insert_coverage && (labelchangemap[oldidx + 1] += 1)
-                    end
-                    if blockchangemap === nothing
-                        blockchangemap = fill(0, length(sv.cfg.blocks))
-                    end
-                    blockchangemap[block] += 1
-                    idx += 1
+                    insert_statement_here!(ReturnNode(), Union{}; after=true)
                 end
                 oldidx = last(sv.cfg.blocks[block].stmts)
             end
@@ -1500,6 +1505,8 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
             cond = el.cond
             if isa(cond, SSAValue)
                 cond = SSAValue(cond.id + ssachangemap[cond.id])
+            elseif isa(cond, NewSSAValue)
+                cond = SSAValue(cond.id)
             end
             was_deleted = labelchangemap[el.dest] == typemin(Int)
             body[i] = was_deleted ? cond : GotoIfNot(cond, el.dest + labelchangemap[el.dest])
@@ -1508,6 +1515,8 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
                 val = el.val
                 if isa(val, SSAValue)
                     body[i] = ReturnNode(SSAValue(val.id + ssachangemap[val.id]))
+                elseif isa(val, NewSSAValue)
+                    body[i] = ReturnNode(SSAValue(val.id))
                 end
             end
         elseif isa(el, SSAValue)
@@ -1527,6 +1536,7 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
                     if isa(val, SSAValue)
                         values[i] = SSAValue(val.id + ssachangemap[val.id])
                     end
+                    @assert !isa(val, NewSSAValue)
                     i += 1
                 end
             end
@@ -1538,8 +1548,14 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
                     @assert !isdefined(el, :scope)
                     body[i] = nothing
                 else
-                    if isdefined(el, :scope) && isa(el.scope, SSAValue)
-                        body[i] = EnterNode(tgt + labelchangemap[tgt], SSAValue(el.scope.id + ssachangemap[el.scope.id]))
+                    if isdefined(el, :scope)
+                        if isa(el.scope, SSAValue)
+                            body[i] = EnterNode(tgt + labelchangemap[tgt], SSAValue(el.scope.id + ssachangemap[el.scope.id]))
+                        elseif isa(el.scope, NewSSAValue)
+                            body[i] = EnterNode(tgt + labelchangemap[tgt], SSAValue(el.scope.id))
+                        else
+                            body[i] = EnterNode(el, tgt + labelchangemap[tgt])
+                        end
                     else
                         body[i] = EnterNode(el, tgt + labelchangemap[tgt])
                     end
@@ -1555,10 +1571,13 @@ function renumber_ir_elements!(body::Vector{Any}, ssachangemap::Vector{Int}, lab
                     el = args[i]
                     if isa(el, SSAValue)
                         args[i] = SSAValue(el.id + ssachangemap[el.id])
+                    elseif isa(el, NewSSAValue)
+                        args[i] = SSAValue(el.id)
                     end
                 end
             end
         end
+        #@assert !isa(el, NewSSAValue)
     end
 end
 
diff --git a/Compiler/src/ssair/irinterp.jl b/Compiler/src/ssair/irinterp.jl
index a4969e81828cc..935cbd7a53ab9 100644
--- a/Compiler/src/ssair/irinterp.jl
+++ b/Compiler/src/ssair/irinterp.jl
@@ -44,6 +44,7 @@ function abstract_eval_invoke_inst(interp::AbstractInterpreter, inst::Instructio
     end
     argtypes = collect_argtypes(interp, stmt.args[2:end], StatementState(nothing, false), irsv)
     argtypes === nothing && return Pair{Any,Tuple{Bool,Bool}}(Bottom, (false, false))
+    (argtypes, _) = argtypes
     return concrete_eval_invoke(interp, code, argtypes, irsv)
 end
 
@@ -308,7 +309,8 @@ populate_def_use_map!(tpdum::TwoPhaseDefUseMap, ir::IRCode) =
 function is_all_const_call(@nospecialize(stmt), interp::AbstractInterpreter, irsv::IRInterpretationState)
     isexpr(stmt, :call) || return false
     @inbounds for i = 2:length(stmt.args)
-        argtype = abstract_eval_value(interp, stmt.args[i], StatementState(nothing, false), irsv)
+        # Discard exct - IRCode semantics require this to be Bottom
+        (argtype, _) = abstract_eval_value(interp, stmt.args[i], StatementState(nothing, false), irsv)
         is_const_argtype(argtype) || return false
     end
     return true
diff --git a/Compiler/src/ssair/verify.jl b/Compiler/src/ssair/verify.jl
index 072a564a31f78..7b043a79377da 100644
--- a/Compiler/src/ssair/verify.jl
+++ b/Compiler/src/ssair/verify.jl
@@ -61,7 +61,7 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
             raise_error()
         end
     elseif isa(op, GlobalRef)
-        if !isdefined(op.mod, op.name) || !isconst(op.mod, op.name)
+        if !is_global_nothrow_const_in_all_worlds(op, ir.valid_worlds)
             @verify_error "Unbound GlobalRef not allowed in value position"
             raise_error()
         end
diff --git a/src/ast.c b/src/ast.c
index 474c0661f5230..959bac26c2c15 100644
--- a/src/ast.c
+++ b/src/ast.c
@@ -181,42 +181,6 @@ static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint
     return (bpart != NULL && decode_restriction_kind(jl_atomic_load_relaxed(&bpart->restriction)) == BINDING_KIND_GLOBAL) ? fl_ctx->T : fl_ctx->F;
 }
 
-static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
-{
-    // tells whether a var is defined, in the sense that accessing it is nothrow
-    // can take either a symbol or a module and a symbol
-    jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
-    jl_module_t *mod = ctx->module;
-    jl_sym_t *var = NULL;
-    if (nargs == 1) {
-        (void)tosymbol(fl_ctx, args[0], "nothrow-julia-global");
-        var = scmsym_to_julia(fl_ctx, args[0]);
-    }
-    else {
-        argcount(fl_ctx, "nothrow-julia-global", nargs, 2);
-        value_t argmod = args[0];
-        if (iscvalue(argmod) && cv_class((cvalue_t*)ptr(argmod)) == jl_ast_ctx(fl_ctx)->jvtype) {
-            mod = *(jl_module_t**)cv_data((cvalue_t*)ptr(argmod));
-            JL_GC_PROMISE_ROOTED(mod);
-        } else {
-            if (!iscons(argmod) || !issymbol(car_(argmod)) || scmsym_to_julia(fl_ctx, car_(argmod)) != jl_thismodule_sym) {
-                lerrorf(fl_ctx, fl_ctx->ArgError, "nothrow-julia-global: Unknown globalref module kind");
-            }
-        }
-        (void)tosymbol(fl_ctx, args[1], "nothrow-julia-global");
-        var = scmsym_to_julia(fl_ctx, args[1]);
-    }
-    jl_binding_t *b = jl_get_module_binding(mod, var, 0);
-    jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age);
-    jl_ptr_kind_union_t pku = jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age);
-    if (!bpart)
-        return fl_ctx->F;
-    if (jl_bkind_is_some_guard(decode_restriction_kind(pku)))
-        return fl_ctx->F;
-    return  (jl_bkind_is_some_constant(decode_restriction_kind(pku)) ?
-        decode_restriction_value(pku) : jl_atomic_load_relaxed(&b->value)) != NULL ? fl_ctx->T : fl_ctx->F;
-}
-
 // Used to generate a unique suffix for a given symbol (e.g. variable or type name)
 // first argument contains a stack of method definitions seen so far by `closure-convert` in flisp.
 // if the top of the stack is non-NIL, we use it to augment the suffix so that it becomes
@@ -288,7 +252,6 @@ static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *m
 
 static const builtinspec_t julia_flisp_ast_ext[] = {
     { "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint
-    { "nothrow-julia-global", fl_nothrow_julia_global },
     { "current-julia-module-counter", fl_module_unique_name },
     { "julia-scalar?", fl_julia_scalar },
     { NULL, NULL }
diff --git a/src/jlfrontend.scm b/src/jlfrontend.scm
index 3d46940d6fcbb..9c69da199c0cd 100644
--- a/src/jlfrontend.scm
+++ b/src/jlfrontend.scm
@@ -31,7 +31,6 @@
 
 ;; this is overwritten when we run in actual julia
 (define (defined-julia-global v) #f)
-(define (nothrow-julia-global v) #f)
 
 ;; parser entry points
 
diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm
index c7ca5d553bb31..d5ac8cca0861e 100644
--- a/src/julia-syntax.scm
+++ b/src/julia-syntax.scm
@@ -4356,7 +4356,7 @@ f(x) = yt(x)
 
 (define (valid-ir-argument? e)
   (or (simple-atom? e)
-      (and (globalref? e) (nothrow-julia-global (cadr e) (caddr e)))
+      (globalref? e)
       (and (pair? e)
            (memq (car e) '(quote inert top core
                                  slot static_parameter)))))
@@ -4534,6 +4534,8 @@ f(x) = yt(x)
                (or (memq aval (lam:args lam))
                    (let ((vi (get vinfo-table aval #f)))
                      (and vi (vinfo:never-undef vi)))))))
+    (define (valid-cond-argument? aval)
+      (and (not (globalref? aval)) (valid-body-ir-argument? aval)))
     (define (single-assign-var? aval)
       (and (symbol? aval) ; Arguments are always sa
            (or (memq aval (lam:args lam))
@@ -4577,7 +4579,7 @@ f(x) = yt(x)
       (let ((cnd (or (compile ex break-labels #t #f)
                      ;; TODO: condition exprs that don't yield a value?
                      '(null))))
-        (if (valid-body-ir-argument? cnd) cnd
+        (if (valid-cond-argument? cnd) cnd
             (let ((tmp (make-ssavalue)))
               (emit `(= ,tmp ,cnd))
               tmp))))
diff --git a/test/core.jl b/test/core.jl
index 39d02d5d567c9..52900fec687cb 100644
--- a/test/core.jl
+++ b/test/core.jl
@@ -7461,23 +7461,22 @@ let code = code_lowered(FieldConvert)[1].code
         field_type2_ssa, field_type4_ssa, field_type5_ssa,
         slot_read_1, slot_read_2, slot_read_3, slot_read_4,
         new_ssa
-    @test code[(fc_global_ssa = 1;)] == GlobalRef(@__MODULE__, :FieldConvert)
-    @test code[(sp1_ssa = 2;)] == Expr(:static_parameter, 1)
-    @test code[(apply_type_ssa = 3;)] == Expr(:call, GlobalRef(Core, :apply_type), Core.SSAValue(fc_global_ssa), GlobalRef(@__MODULE__, :FieldTypeA), Core.SSAValue(sp1_ssa))
-    @test code[(field_type_ssa = 4;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 1)
-    @test code[10] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type_ssa), Core.SlotNumber(10)))
-    @test code[(slot_read_1 = 11;)] == Core.SlotNumber(10)
-    @test code[(field_type2_ssa = 12;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 2)
-    @test code[18] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type2_ssa), Core.SlotNumber(9)))
-    @test code[(slot_read_2 = 19;)] == Core.SlotNumber(9)
-    @test code[(field_type4_ssa = 20;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 4)
-    @test code[26] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type4_ssa), Core.SlotNumber(8)))
-    @test code[(slot_read_3 = 27;)] == Core.SlotNumber(8)
-    @test code[(field_type5_ssa = 28;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 5)
-    @test code[34] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type5_ssa), Core.SlotNumber(7)))
-    @test code[(slot_read_4 = 35;)] == Core.SlotNumber(7)
-    @test code[(new_ssa = 36;)] == Expr(:new, Core.SSAValue(apply_type_ssa), Core.SSAValue(slot_read_1), Core.SSAValue(slot_read_2), Core.SlotNumber(4), Core.SSAValue(slot_read_3), Core.SSAValue(slot_read_4))
-    @test code[37] == Core.ReturnNode(Core.SSAValue(new_ssa))
+    @test code[(sp1_ssa = 1;)] == Expr(:static_parameter, 1)
+    @test code[(apply_type_ssa = 2;)] == Expr(:call, GlobalRef(Core, :apply_type), GlobalRef(@__MODULE__, :FieldConvert), GlobalRef(@__MODULE__, :FieldTypeA), Core.SSAValue(sp1_ssa))
+    @test code[(field_type_ssa = 3;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 1)
+    @test code[9] == Expr(:(=), Core.SlotNumber(10), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type_ssa), Core.SlotNumber(10)))
+    @test code[(slot_read_1 = 10;)] == Core.SlotNumber(10)
+    @test code[(field_type2_ssa = 11;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 2)
+    @test code[17] == Expr(:(=), Core.SlotNumber(9), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type2_ssa), Core.SlotNumber(9)))
+    @test code[(slot_read_2 = 18;)] == Core.SlotNumber(9)
+    @test code[(field_type4_ssa = 19;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 4)
+    @test code[25] == Expr(:(=), Core.SlotNumber(8), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type4_ssa), Core.SlotNumber(8)))
+    @test code[(slot_read_3 = 26;)] == Core.SlotNumber(8)
+    @test code[(field_type5_ssa = 27;)] == Expr(:call, GlobalRef(Core, :fieldtype), Core.SSAValue(apply_type_ssa), 5)
+    @test code[33] == Expr(:(=), Core.SlotNumber(7), Expr(:call, GlobalRef(Base, :convert), Core.SSAValue(field_type5_ssa), Core.SlotNumber(7)))
+    @test code[(slot_read_4 = 34;)] == Core.SlotNumber(7)
+    @test code[(new_ssa = 35;)] == Expr(:new, Core.SSAValue(apply_type_ssa), Core.SSAValue(slot_read_1), Core.SSAValue(slot_read_2), Core.SlotNumber(4), Core.SSAValue(slot_read_3), Core.SSAValue(slot_read_4))
+    @test code[36] == Core.ReturnNode(Core.SSAValue(new_ssa))
 end
 
 # Issue #32820