From f4ced3ed2efbf33849d7b849438ec8ed3aa24a96 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Thu, 1 Feb 2024 19:53:40 +0000 Subject: [PATCH 1/6] Fix interpreter_exec.jl test This test was supposed to check that we correctly handled PhiNodes in uninferred code in both the interpreter and the compiler. However, the compiler path wasn't actually exercised, because the `inferred=true` part of this forced it to be skipped. Drop that and fix the exposed issues in the compiler where we didn't handle PhiNodes properly. --- base/compiler/abstractinterpretation.jl | 4 ++++ base/compiler/optimize.jl | 2 +- test/compiler/interpreter_exec.jl | 10 ++++------ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 2b736dbeff9d8..ad07e68151d8f 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -3304,6 +3304,10 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState) end if rt === Bottom ssavaluetypes[currpc] = Bottom + # Special case: Union typed PhiNodes do not error (but must also be unused) + if isa(stmt, PhiNode) + continue + end @goto find_next_bb end if changes !== nothing diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index f634fc0bcb7e4..cbe80a65fc607 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -1086,7 +1086,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState) idx += 1 prevloc = codeloc end - if ssavaluetypes[idx] === Union{} && !(oldidx in sv.unreachable) + 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]) diff --git a/test/compiler/interpreter_exec.jl b/test/compiler/interpreter_exec.jl index ce0704be15178..2e5fa09dd9a9e 100644 --- a/test/compiler/interpreter_exec.jl +++ b/test/compiler/interpreter_exec.jl @@ -20,10 +20,9 @@ let m = Meta.@lower 1 + 1 ReturnNode(SSAValue(6)), ] nstmts = length(src.code) - src.ssavaluetypes = Any[ Any for _ = 1:nstmts ] + src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) - src.inferred = true Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test :a === @eval $m @@ -61,13 +60,13 @@ let m = Meta.@lower 1 + 1 ReturnNode(SSAValue(18)), ] nstmts = length(src.code) - src.ssavaluetypes = Any[ Any for _ = 1:nstmts ] + src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) - src.inferred = true Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test (:b, :a, :c, :c) === @eval $m + src.ssavaluetypes = nstmts global test29262 = false @test (:b, :a, :c, :b) === @eval $m end @@ -98,10 +97,9 @@ let m = Meta.@lower 1 + 1 ReturnNode(SSAValue(11)), ] nstmts = length(src.code) - src.ssavaluetypes = Any[ Any for _ = 1:nstmts ] + src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) - src.inferred = true Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test :a === @eval $m From b8c24bb43717884e5cf1a02f4912c7f7f9c9494a Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Thu, 8 Feb 2024 19:30:58 +0900 Subject: [PATCH 2/6] handle unassigned `PhiNode` values in `find_ssavalue_uses` --- base/compiler/utilities.jl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/base/compiler/utilities.jl b/base/compiler/utilities.jl index 368395e714054..806ad4a0a2464 100644 --- a/base/compiler/utilities.jl +++ b/base/compiler/utilities.jl @@ -415,15 +415,15 @@ function find_ssavalue_uses(body::Vector{Any}, nvals::Int) if isa(e, SSAValue) push!(uses[e.id], line) elseif isa(e, Expr) - find_ssavalue_uses(e, uses, line) + find_ssavalue_uses!(e, uses, line) elseif isa(e, PhiNode) - find_ssavalue_uses(e, uses, line) + find_ssavalue_uses!(e, uses, line) end end return uses end -function find_ssavalue_uses(e::Expr, uses::Vector{BitSet}, line::Int) +function find_ssavalue_uses!(e::Expr, uses::Vector{BitSet}, line::Int) head = e.head is_meta_expr_head(head) && return skiparg = (head === :(=)) @@ -433,13 +433,16 @@ function find_ssavalue_uses(e::Expr, uses::Vector{BitSet}, line::Int) elseif isa(a, SSAValue) push!(uses[a.id], line) elseif isa(a, Expr) - find_ssavalue_uses(a, uses, line) + find_ssavalue_uses!(a, uses, line) end end end -function find_ssavalue_uses(e::PhiNode, uses::Vector{BitSet}, line::Int) - for val in e.values +function find_ssavalue_uses!(e::PhiNode, uses::Vector{BitSet}, line::Int) + values = e.values + for i = 1:length(values) + isassigned(values) || continue + val = values[i] if isa(val, SSAValue) push!(uses[val.id], line) end From 16703daa20cf623b5e1e5a75ef315bf6b02f2044 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 8 Feb 2024 13:18:11 -0500 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> --- base/compiler/utilities.jl | 2 +- test/compiler/interpreter_exec.jl | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/base/compiler/utilities.jl b/base/compiler/utilities.jl index 806ad4a0a2464..eec64307c533b 100644 --- a/base/compiler/utilities.jl +++ b/base/compiler/utilities.jl @@ -441,7 +441,7 @@ end function find_ssavalue_uses!(e::PhiNode, uses::Vector{BitSet}, line::Int) values = e.values for i = 1:length(values) - isassigned(values) || continue + isassigned(values, i) || continue val = values[i] if isa(val, SSAValue) push!(uses[val.id], line) diff --git a/test/compiler/interpreter_exec.jl b/test/compiler/interpreter_exec.jl index 2e5fa09dd9a9e..792b9eb65ca28 100644 --- a/test/compiler/interpreter_exec.jl +++ b/test/compiler/interpreter_exec.jl @@ -23,6 +23,7 @@ let m = Meta.@lower 1 + 1 src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) + @assert !src.inferred Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test :a === @eval $m From ab74bc2b99b35c027130c547dea31d7965fb39c9 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Thu, 8 Feb 2024 13:18:48 -0500 Subject: [PATCH 4/6] Update base/compiler/abstractinterpretation.jl --- base/compiler/abstractinterpretation.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index ad07e68151d8f..9244ee5f3f8ec 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -3304,7 +3304,7 @@ function typeinf_local(interp::AbstractInterpreter, frame::InferenceState) end if rt === Bottom ssavaluetypes[currpc] = Bottom - # Special case: Union typed PhiNodes do not error (but must also be unused) + # Special case: Bottom-typed PhiNodes do not error (but must also be unused) if isa(stmt, PhiNode) continue end From d056498e3bf9c51737c775b787d68c17a9d89a04 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:02:47 +0900 Subject: [PATCH 5/6] refactors `interpreter_exec.jl` further (#53246) This commit refines `interpreter_exec.jl` further, on top of #53218. In detail, by using `Base.Experimental.@compiler_options compile=min`, it's now possible to mimic the effect of `julia --compile=[min|yes]`. This commit is leveraging it and includes `interpreter_exec.jl` from `test/compiler/ssair.jl`, allowing us to get better stack traces and test summaries. In addition, I've introduced several tests concerning the `src.inferred` values and have relocated the tests from #47066 within `interpreter_exec.jl` to `test/compiler/irpasses.jl`, since it is related to Julia-level compilation and not to interpreter execution. --- test/compiler/interpreter_exec.jl | 33 +++++++++++++++++-------------- test/compiler/irpasses.jl | 14 +++++++++++++ test/compiler/ssair.jl | 14 ++++++++----- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/test/compiler/interpreter_exec.jl b/test/compiler/interpreter_exec.jl index 792b9eb65ca28..31f3f510cf4a2 100644 --- a/test/compiler/interpreter_exec.jl +++ b/test/compiler/interpreter_exec.jl @@ -23,10 +23,15 @@ let m = Meta.@lower 1 + 1 src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) - @assert !src.inferred + @test !src.inferred Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test :a === @eval $m + compile_mode = @ccall jl_get_module_compile(@__MODULE__()::Module)::Cint + if compile_mode == 3 + # implies `Base.Experimental.@compiler_options compile=min` + @test !src.inferred + end global test29262 = false @test :b === @eval $m end @@ -64,9 +69,15 @@ let m = Meta.@lower 1 + 1 src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) + @test !src.inferred Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test (:b, :a, :c, :c) === @eval $m + compile_mode = @ccall jl_get_module_compile(@__MODULE__()::Module)::Cint + if compile_mode == 3 + # implies `Base.Experimental.@compiler_options compile=min` + @test !src.inferred + end src.ssavaluetypes = nstmts global test29262 = false @test (:b, :a, :c, :b) === @eval $m @@ -101,23 +112,15 @@ let m = Meta.@lower 1 + 1 src.ssavaluetypes = nstmts src.ssaflags = fill(UInt8(0x00), nstmts) src.codelocs = fill(Int32(1), nstmts) + @test !src.inferred Core.Compiler.verify_ir(Core.Compiler.inflate_ir(src)) global test29262 = true @test :a === @eval $m + compile_mode = @ccall jl_get_module_compile(@__MODULE__()::Module)::Cint + if compile_mode == 3 + # implies `Base.Experimental.@compiler_options compile=min` + @test !src.inferred + end global test29262 = false @test :b === @eval $m end - -# https://github.com/JuliaLang/julia/issues/47065 -# `Core.Compiler.sort!` should be able to handle a big list -let n = 1000 - ex = :(return 1) - for _ in 1:n - ex = :(rand() < .1 && $(ex)) - end - @eval global function f_1000_blocks() - $ex - return 0 - end -end -@test f_1000_blocks() == 0 diff --git a/test/compiler/irpasses.jl b/test/compiler/irpasses.jl index 2e14fdb8d3013..9345c2e26db33 100644 --- a/test/compiler/irpasses.jl +++ b/test/compiler/irpasses.jl @@ -1787,3 +1787,17 @@ let code = Any[ @test !any(iscall((ir, getfield)), ir.stmts.stmt) @test length(ir.cfg.blocks[end].stmts) == 1 end + +# https://github.com/JuliaLang/julia/issues/47065 +# `Core.Compiler.sort!` should be able to handle a big list +let n = 1000 + ex = :(return 1) + for _ in 1:n + ex = :(rand() < .1 && $(ex)) + end + @eval global function f_1000_blocks() + $ex + return 0 + end +end +@test f_1000_blocks() == 0 diff --git a/test/compiler/ssair.jl b/test/compiler/ssair.jl index bab11d3ead522..3a90ee6308d53 100644 --- a/test/compiler/ssair.jl +++ b/test/compiler/ssair.jl @@ -92,11 +92,15 @@ let cfg = CFG(BasicBlock[ end end -for compile in ("min", "yes") - cmd = `$(Base.julia_cmd()) --compile=$compile interpreter_exec.jl` - if !success(pipeline(Cmd(cmd, dir=@__DIR__); stdout=stdout, stderr=stderr)) - error("Interpreter test failed, cmd : $cmd") - end +# test code execution with the default compile-mode +module CompilerExecTest +include("interpreter_exec.jl") +end + +# test code execution with the interpreter mode (compile=min) +module InterpreterExecTest +Base.Experimental.@compiler_options compile=min +include("interpreter_exec.jl") end # PR #32145 From 750948b511fbfde121d53d3a6fc55d32071e0b50 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Fri, 9 Feb 2024 11:06:53 +0900 Subject: [PATCH 6/6] better argument order --- base/compiler/utilities.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/compiler/utilities.jl b/base/compiler/utilities.jl index eec64307c533b..00c7ea40e3db9 100644 --- a/base/compiler/utilities.jl +++ b/base/compiler/utilities.jl @@ -415,15 +415,15 @@ function find_ssavalue_uses(body::Vector{Any}, nvals::Int) if isa(e, SSAValue) push!(uses[e.id], line) elseif isa(e, Expr) - find_ssavalue_uses!(e, uses, line) + find_ssavalue_uses!(uses, e, line) elseif isa(e, PhiNode) - find_ssavalue_uses!(e, uses, line) + find_ssavalue_uses!(uses, e, line) end end return uses end -function find_ssavalue_uses!(e::Expr, uses::Vector{BitSet}, line::Int) +function find_ssavalue_uses!(uses::Vector{BitSet}, e::Expr, line::Int) head = e.head is_meta_expr_head(head) && return skiparg = (head === :(=)) @@ -433,12 +433,12 @@ function find_ssavalue_uses!(e::Expr, uses::Vector{BitSet}, line::Int) elseif isa(a, SSAValue) push!(uses[a.id], line) elseif isa(a, Expr) - find_ssavalue_uses!(a, uses, line) + find_ssavalue_uses!(uses, a, line) end end end -function find_ssavalue_uses!(e::PhiNode, uses::Vector{BitSet}, line::Int) +function find_ssavalue_uses!(uses::Vector{BitSet}, e::PhiNode, line::Int) values = e.values for i = 1:length(values) isassigned(values, i) || continue