reland "Inlining: Remove outdated code path for GlobalRef movement (#…

…46880)" (#56382)

From the description of the original PR:
> We used to not allow `GlobalRef` in `PhiNode` at all (because they
> could have side effects). However, we then change the IR to make
> side-effecting `GlobalRef`s illegal in statement position in general,
> so now `PhiNode`s values are just regular value position, so there's
> no reason any more to try to move `GlobalRef`s out to statement
> position in inlining. Moreover, doing so introduces a bunch of
> unnecessary `GlobalRef`s that weren't being moved back. We could fix
> that separately by setting appropriate flags, but it's simpler to just
> get rid of this special case entirely.

This change itself does not sound to have any issues, and in fact, it is
very useful for keeping the IR slim, especially in code generated by
Cassette-like systems, so I would like to reland it.

However, the original PR was reverted in #46951 due to
bugs like #46940 and #46943. I could not
reproduce these bugs on my end (maybe they have been fixed on some
GC-side fixes?), so I believe relanding the original PR’s changes would
not cause any issues, but it is necessary to confirm that similar
problems do not arise before merging this PR.

base/compiler/ssair/inlining.jl

@@ -665,18 +665,6 @@ function batch_inline!(ir::IRCode, todo::Vector{Pair{Int,Any}}, propagate_inboun
                     compact.active_result_bb -= 1
                     refinish = true
                 end
-                # It is possible for GlobalRefs and Exprs to be in argument position
-                # at this point in the IR, though in that case they are required
-                # to be effect-free. However, we must still move them out of argument
-                # position, since `Argument` is allowed in PhiNodes, but `GlobalRef`
-                # and `Expr` are not, so a substitution could anger the verifier.
-                for aidx in 1:length(argexprs)
-                    aexpr = argexprs[aidx]
-                    if isa(aexpr, Expr) || isa(aexpr, GlobalRef)
-                        ninst = removable_if_unused(NewInstruction(aexpr, argextype(aexpr, compact), compact.result[idx][:line]))
-                        argexprs[aidx] = insert_node_here!(compact, ninst)
-                    end
-                end
                 if isa(item, InliningTodo)
                     compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, item, boundscheck, state.todo_bbs)
                 elseif isa(item, UnionSplit)

test/compiler/EscapeAnalysis/EscapeAnalysis.jl

@@ -671,8 +671,8 @@ end
         @test has_all_escape(result.state[Argument(3)]) # b
     end
     let result = @eval EATModule() begin
-            const Rx = SafeRef{String}("Rx")
-            $code_escapes((String,)) do s
+            const Rx = SafeRef(Ref(""))
+            $code_escapes((Base.RefValue{String},)) do s
                 Rx[] = s
                 Core.sizeof(Rx[])
             end

test/compiler/inline.jl

@@ -2282,3 +2282,10 @@ let;Base.Experimental.@force_compile
     f_EA_finalizer(42000)
     @test foreign_buffer_checker.finalized
 end
+
+# Test that inlining doesn't unnecessarily move things to statement position
+@noinline f_noinline_invoke(x::Union{Symbol,Nothing}=nothing) = Core.donotdelete(x)
+g_noinline_invoke(x) = f_noinline_invoke(x)
+let src = code_typed1(g_noinline_invoke, (Union{Symbol,Nothing},))
+    @test !any(@nospecialize(x)->isa(x,GlobalRef), src.code)
+end