Merge branch 'master' into dpa/test-nanosoldier-stdlibs

Author: DilumAluthge

AGENTS.md

@@ -40,6 +40,11 @@ and need to run code with these changes included, you can use `Revise`.
 To do so, run `using Revise; Revise.track(Base)` (or Revise.track with the stdlib you modified).
 The test system supports doing this automatically (see below).
 
+For instance testing Base changes without rebuilding, using failfast, you can run:
+```
+JULIA_TEST_FAILFAST=1 ./julia -e 'using Revise; Revise.track(Base); include("test.jl")'
+```
+
 ## Specific instructions for particular changes
 
 ### Doctests

Compiler/src/abstractinterpretation.jl

@@ -2485,7 +2485,7 @@ function abstract_eval_setglobal!(interp::AbstractInterpreter, sv::AbsIntState,
             (rt, exct) = global_assignment_rt_exct(interp, sv, saw_latestworld, gr, v)
             return CallMeta(rt, exct, Effects(setglobal!_effects, nothrow=exct===Bottom), GlobalAccessInfo(convert(Core.Binding, gr)))
         end
-        return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo())
+        return CallMeta(Union{}, Union{TypeError, ErrorException}, EFFECTS_THROWS, NoCallInfo())
     end
     ⊑ = partialorder(typeinf_lattice(interp))
     if !(hasintersect(widenconst(M), Module) && hasintersect(widenconst(s), Symbol))
@@ -3480,65 +3480,33 @@ function refine_partial_type(@nospecialize t)
     return t
 end
 
-abstract_eval_nonlinearized_foreigncall_name(
-        ::AbstractInterpreter, @nospecialize(e), ::StatementState, ::IRInterpretationState
-    ) = nothing
-
-function abstract_eval_nonlinearized_foreigncall_name(
-        interp::AbstractInterpreter, @nospecialize(e), sstate::StatementState, sv::InferenceState
-    )
-    if isexpr(e, :call)
-        n = length(e.args)
-        argtypes = Vector{Any}(undef, n)
-        callresult = Future{CallMeta}()
-        i::Int = 1
-        nextstate::UInt8 = 0x0
-        local ai, res
-        function evalargs(interp, sv)
-            if nextstate === 0x1
-                @goto state1
-            elseif nextstate === 0x2
-                @goto state2
-            end
-            while i <= n
-                ai = abstract_eval_nonlinearized_foreigncall_name(interp, e.args[i], sstate, sv)
-                if !isready(ai)
-                    nextstate = 0x1
-                    return false
-                    @label state1
-                end
-                argtypes[i] = ai[].rt
-                i += 1
-            end
-            res = abstract_call(interp, ArgInfo(e.args, argtypes), sstate, sv)
-            if !isready(res)
-                nextstate = 0x2
-                return false
-                @label state2
-            end
-            callresult[] = res[]
-            return true
-        end
-        evalargs(interp, sv) || push!(sv.tasks, evalargs)
-        return callresult
-    else
-        return Future(abstract_eval_basic_statement(interp, e, sstate, sv))
-    end
-end
-
 function abstract_eval_foreigncall(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState)
     callee = e.args[1]
-    if isexpr(callee, :call) && length(callee.args) > 1 && callee.args[1] == GlobalRef(Core, :tuple)
-        # NOTE these expressions are not properly linearized
-        abstract_eval_nonlinearized_foreigncall_name(interp, callee.args[2], sstate, sv)
-        if length(callee.args) > 2
-            abstract_eval_nonlinearized_foreigncall_name(interp, callee.args[3], sstate, sv)
+    if isexpr(callee, :tuple)
+        if length(callee.args) >= 1
+            # Evaluate the arguments to constrain the world, effects, and other info for codegen,
+            # but note there is an implied `if !=(C_NULL)` branch here that might read data
+            # in a different world (the exact cache behavior is unspecified), so we do not use
+            # these results to refine reachability of the subsequent foreigncall.
+            abstract_eval_value(interp, callee.args[1], sstate, sv)
+            if length(callee.args) >= 2
+                abstract_eval_value(interp, callee.args[2], sstate, sv)
+                #TODO: implement abstract_eval_nonlinearized_foreigncall_name correctly?
+                # lib_effects = abstract_call(interp, ArgInfo(e.args, Any[typeof(Libdl.dlopen), lib]), sstate, sv)::Future
+            end
         end
     else
         abstract_eval_value(interp, callee, sstate, sv)
     end
     mi = frame_instance(sv)
     t = sp_type_rewrap(e.args[2], mi, true)
+    let fptr = e.args[1]
+        if !isexpr(fptr, :tuple)
+            if !hasintersect(widenconst(abstract_eval_value(interp, fptr, sstate, sv)), Ptr)
+                return RTEffects(Bottom, Any, EFFECTS_THROWS)
+            end
+        end
+    end
     for i = 3:length(e.args)
         if abstract_eval_value(interp, e.args[i], sstate, sv) === Bottom
             return RTEffects(Bottom, Any, EFFECTS_THROWS)
@@ -3751,7 +3719,7 @@ end
 
 function global_assignment_rt_exct(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, g::GlobalRef, @nospecialize(newty))
     if saw_latestworld
-        return Pair{Any,Any}(newty, ErrorException)
+        return Pair{Any,Any}(newty, Union{TypeError, ErrorException})
     end
     newty′ = RefValue{Any}(newty)
     (valid_worlds, ret) = scan_partitions(interp, g, sv.world) do interp::AbstractInterpreter, ::Core.Binding, partition::Core.BindingPartition
@@ -3766,15 +3734,16 @@ function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partitio
     if is_some_guard(kind)
         return Pair{Any,Any}(newty, ErrorException)
     elseif is_some_const_binding(kind) || is_some_imported(kind)
-        return Pair{Any,Any}(Bottom, ErrorException)
+        # N.B.: Backdating should not improve inference in an earlier world
+        return Pair{Any,Any}(kind == PARTITION_KIND_BACKDATED_CONST ? newty : Bottom, ErrorException)
     end
     ty = kind == PARTITION_KIND_DECLARED ? Any : partition_restriction(partition)
     wnewty = widenconst(newty)
     if !hasintersect(wnewty, ty)
-        return Pair{Any,Any}(Bottom, ErrorException)
+        return Pair{Any,Any}(Bottom, TypeError)
     elseif !(wnewty <: ty)
         retty = tmeet(typeinf_lattice(interp), newty, ty)
-        return Pair{Any,Any}(retty, ErrorException)
+        return Pair{Any,Any}(retty, TypeError)
     end
     return Pair{Any,Any}(newty, Bottom)
 end

Compiler/src/optimize.jl

@@ -861,10 +861,14 @@ function scan_inconsistency!(inst::Instruction, sv::PostOptAnalysisState)
     # Special case: For `getfield` and memory operations, we allow inconsistency of the :boundscheck argument
     (; inconsistent, tpdum) = sv
     if iscall_with_boundscheck(stmt, sv)
-        for i = 1:(length(stmt.args)-1)
+        for i = 1:length(stmt.args)
             val = stmt.args[i]
+            # SSAValue should be the only permitted argument type which can be inconsistent found here.
+            # Others (e.g. GlobalRef) should have been moved to statement position. See stmt_effect_flags.
             if isa(val, SSAValue)
-                stmt_inconsistent |= val.id in inconsistent
+                if i < length(stmt.args)  # not the boundscheck argument (which is last)
+                    stmt_inconsistent |= val.id in inconsistent
+                end
                 count!(tpdum, val)
             end
         end
@@ -891,7 +895,7 @@ function ((; sv)::ScanStmt)(inst::Instruction, lstmt::Int, bb::Int)
     if isa(stmt, EnterNode)
         # try/catch not yet modeled
         give_up_refinements!(sv)
-        return nothing
+        return true # don't bail out early -- can cause tpdum counts to be off
     end
 
     scan_non_dataflow_flags!(inst, sv)
@@ -937,36 +941,36 @@ function ((; sv)::ScanStmt)(inst::Instruction, lstmt::Int, bb::Int)
         end
     end
 
-    # bail out early if there are no possibilities to refine the effects
-    if !any_refinable(sv)
-        return nothing
-    end
+    # Do not bail out early, as this can cause tpdum counts to be off.
+    # # bail out early if there are no possibilities to refine the effects
+    # if !any_refinable(sv)
+    #     return nothing
+    # end
 
     return true
 end
 
 function check_inconsistentcy!(sv::PostOptAnalysisState, scanner::BBScanner)
     (; ir, inconsistent, tpdum) = sv
 
+    sv.all_retpaths_consistent || return
     scan!(ScanStmt(sv), scanner, false)
+    sv.all_retpaths_consistent || return
     complete!(tpdum); push!(scanner.bb_ip, 1)
     populate_def_use_map!(tpdum, scanner)
 
     stmt_ip = BitSetBoundedMinPrioritySet(length(ir.stmts))
     for def in inconsistent
-        for use in tpdum[def]
-            if !(use in inconsistent)
-                push!(inconsistent, use)
-                append!(stmt_ip, tpdum[use])
-            end
-        end
-    end
+        append!(stmt_ip, tpdum[def])
+   end
     lazydomtree = LazyDomtree(ir)
     while !isempty(stmt_ip)
         idx = popfirst!(stmt_ip)
+        idx in inconsistent && continue # already processed
         inst = ir[SSAValue(idx)]
         stmt = inst[:stmt]
         if iscall_with_boundscheck(stmt, sv)
+            # recompute inconsistent flags for call while skipping boundscheck (last) argument
             any_non_boundscheck_inconsistent = false
             for i = 1:(length(stmt.args)-1)
                 val = stmt.args[i]
@@ -978,19 +982,18 @@ function check_inconsistentcy!(sv::PostOptAnalysisState, scanner::BBScanner)
             any_non_boundscheck_inconsistent || continue
         elseif isa(stmt, ReturnNode)
             sv.all_retpaths_consistent = false
+            return
         elseif isa(stmt, GotoIfNot)
             bb = block_for_inst(ir, idx)
             cfg = ir.cfg
             blockliveness = BlockLiveness(cfg.blocks[bb].succs, nothing)
             for succ in iterated_dominance_frontier(cfg, blockliveness, get!(lazydomtree))
                 visit_bb_phis!(ir, succ) do phiidx::Int
-                    push!(inconsistent, phiidx)
-                    push!(stmt_ip, phiidx)
+                    phiidx in inconsistent || push!(stmt_ip, phiidx)
                 end
             end
         end
-        sv.all_retpaths_consistent || break
-        append!(inconsistent, tpdum[idx])
+        push!(inconsistent, idx)
         append!(stmt_ip, tpdum[idx])
     end
 end
@@ -1009,9 +1012,9 @@ function ipo_dataflow_analysis!(interp::AbstractInterpreter, opt::OptimizationSt
     completed_scan = scan!(ScanStmt(sv), scanner, true)
 
     if !completed_scan
-        if sv.all_retpaths_consistent
-            check_inconsistentcy!(sv, scanner)
-        else
+        # finish scanning for all_retpaths_consistent computation
+        check_inconsistentcy!(sv, scanner)
+        if !sv.all_retpaths_consistent
             # No longer any dataflow concerns, just scan the flags
             scan!(scanner, false) do inst::Instruction, ::Int, ::Int
                 scan_non_dataflow_flags!(inst, sv)
@@ -1439,8 +1442,11 @@ function statement_cost(ex::Expr, line::Int, src::Union{CodeInfo, IRCode}, sptyp
         return params.inline_nonleaf_penalty
     elseif head === :foreigncall
         foreigncall = ex.args[1]
-        if foreigncall isa QuoteNode && foreigncall.value === :jl_string_ptr
-            return 1
+        if isexpr(foreigncall, :tuple, 1)
+            foreigncall = foreigncall.args[1]
+            if foreigncall isa QuoteNode && foreigncall.value === :jl_string_ptr
+                return 1
+            end
         end
         return 20
     elseif head === :invoke || head === :invoke_modify

Compiler/src/ssair/EscapeAnalysis.jl

@@ -1037,8 +1037,10 @@ function escape_foreigncall!(astate::AnalysisState, pc::Int, args::Vector{Any})
     # NOTE array allocations might have been proven as nothrow (https://github.com/JuliaLang/julia/pull/43565)
     nothrow = is_nothrow(astate.ir, pc)
     name_info = nothrow ? ⊥ : ThrownEscape(pc)
-    add_escape_change!(astate, name, name_info)
-    add_liveness_change!(astate, name, pc)
+    if !isexpr(name, :tuple)
+        add_escape_change!(astate, name, name_info)
+        add_liveness_change!(astate, name, pc)
+    end
     for i = 1:nargs
         # we should escape this argument if it is directly called,
         # otherwise just impose ThrownEscape if not nothrow

Compiler/src/ssair/inlining.jl

@@ -1753,7 +1753,7 @@ function late_inline_special_case!(ir::IRCode, idx::Int, stmt::Expr, flag::UInt3
             length(stmt.args) == 2 ? Any : stmt.args[end])
         return SomeCase(typevar_call)
     elseif f === UnionAll && length(argtypes) == 3 && ⊑(optimizer_lattice(state.interp), argtypes[2], TypeVar)
-        unionall_call = Expr(:foreigncall, QuoteNode(:jl_type_unionall), Any, svec(Any, Any),
+        unionall_call = Expr(:foreigncall, Expr(:tuple, QuoteNode(:jl_type_unionall)), Any, svec(Any, Any),
             0, QuoteNode(:ccall), stmt.args[2], stmt.args[3])
         return SomeCase(unionall_call)
     elseif is_return_type(f)

Compiler/src/ssair/irinterp.jl

@@ -294,7 +294,7 @@ end
 
 function populate_def_use_map!(tpdum::TwoPhaseDefUseMap, scanner::BBScanner)
     scan!(scanner, false) do inst::Instruction, lstmt::Int, bb::Int
-        for ur in userefs(inst)
+        for ur in userefs(inst[:stmt])
             val = ur[]
             if isa(val, SSAValue)
                 push!(tpdum[val.id], inst.idx)

Compiler/src/ssair/verify.jl

@@ -77,10 +77,10 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
         end
     elseif isa(op, Expr)
         # Only Expr(:boundscheck) is allowed in value position
-        if isforeigncall && arg_idx == 1 && op.head === :call
-            # Allow a tuple in symbol position for foreigncall - this isn't actually
-            # a real call - it's interpreted in global scope by codegen. However,
-            # we do need to keep this a real use, because it could also be a pointer.
+        if isforeigncall && arg_idx == 1 && op.head === :tuple
+            # Allow a tuple literal in symbol position for foreigncall - this
+            # is syntax for a literal value or globalref - it is interpreted in
+            # global scope by codegen.
         elseif !is_value_pos_expr_head(op.head)
             if !allow_frontend_forms || op.head !== :opaque_closure_method
                 @verify_error "Expr not allowed in value position"

Compiler/src/verifytrim.jl

@@ -279,9 +279,17 @@ function verify_codeinstance!(interp::NativeInterpreter, codeinst::CodeInstance,
             error = "unresolved cfunction"
         elseif isexpr(stmt, :foreigncall)
             foreigncall = stmt.args[1]
-            if foreigncall isa QuoteNode
-                if foreigncall.value in runtime_functions
-                    error = "disallowed ccall into a runtime function"
+            if isexpr(foreigncall, :tuple, 1)
+                foreigncall = foreigncall.args[1]
+                if foreigncall isa String
+                    foreigncall = QuoteNode(Symbol(foreigncall))
+                end
+                if foreigncall isa QuoteNode
+                    if foreigncall.value in runtime_functions
+                        error = "disallowed ccall into a runtime function"
+                    end
+                else
+                    error = "disallowed ccall with non-constant name and no library"
                 end
             end
         elseif isexpr(stmt, :new_opaque_closure)

Compiler/test/inference.jl

@@ -5219,11 +5219,11 @@ end
 @testset "#45956: non-linearized cglobal needs special treatment for stmt effects" begin
     function foo()
         cglobal((a, ))
-        ccall(0, Cvoid, (Nothing,), b)
+        ccall(C_NULL, Cvoid, (Nothing,), b)
     end
     @test only(code_typed() do
         cglobal((a, ))
-        ccall(0, Cvoid, (Nothing,), b)
+        ccall(C_NULL, Cvoid, (Nothing,), b)
     end)[2] === Nothing
 end
 
@@ -6458,7 +6458,7 @@ end
 global invalid_setglobal!_exct_modeling::Int
 @test Base.infer_exception_type((Float64,)) do x
     setglobal!(@__MODULE__, :invalid_setglobal!_exct_modeling, x)
-end == ErrorException
+end == TypeError
 
 # Issue #58257 - Hang in inference during BindingPartition resolution
 module A58257

Compiler/test/irpasses.jl

@@ -1071,7 +1071,7 @@ let # Test for https://github.com/JuliaLang/julia/issues/43402
     end
 
     refs = map(Core.SSAValue, findall(@nospecialize(x)->Meta.isexpr(x, :new), src.code))
-    some_ccall = findfirst(@nospecialize(x) -> Meta.isexpr(x, :foreigncall) && x.args[1] == :(:some_ccall), src.code)
+    some_ccall = findfirst(@nospecialize(x) -> Meta.isexpr(x, :foreigncall) && x.args[1] == Expr(:tuple, :(:some_ccall)), src.code)
     @assert some_ccall !== nothing
     stmt = src.code[some_ccall]
     nccallargs = length(stmt.args[3]::Core.SimpleVector)