Skip to content

Commit

Permalink
Tighter array eltype for renumber_ssa (#37499)
Browse files Browse the repository at this point in the history
Unlike `renumber_ssa2` in `compiler/ir.jl` which actually takes an array with elements of
many different types, all callers of `renumber_ssa` (and `renumber_ssa!`) only ever assign
`SSAValue` to the array. Also no one ever checks `isassigned` on this array
so a `Vector{SSAValue}` should work just fine here.

Try to maintain a similar level of error checking by replacing `#undef` with `SSAValue(-1)`
(already used by `construct_ssa!`) and adding an assertion in `renumber_ssa`
to check for cases that throws `UndefVarError` previously.

Also removed an unused parameter.
  • Loading branch information
yuyichao authored Sep 14, 2020
1 parent 1ad4ada commit a0a68a5
Showing 1 changed file with 11 additions and 10 deletions.
21 changes: 11 additions & 10 deletions base/compiler/ssair/slot2ssa.jl
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,19 @@ function scan_slot_def_use(nargs::Int, ci::CodeInfo, code::Vector{Any})
result
end

function renumber_ssa(stmt::SSAValue, ssanums::Vector{Any}, new_ssa::Bool=false, used_ssa::Union{Nothing, Vector{Int}}=nothing)
function renumber_ssa(stmt::SSAValue, ssanums::Vector{SSAValue}, new_ssa::Bool=false)
id = stmt.id
if id > length(ssanums)
return stmt
end
val = ssanums[id]
if isa(val, SSAValue) && used_ssa !== nothing
used_ssa[val.id] += 1
end
@assert val.id > 0
return val
end

function renumber_ssa!(@nospecialize(stmt), ssanums::Vector{Any}, new_ssa::Bool=false, used_ssa::Union{Nothing, Vector{Int}}=nothing)
isa(stmt, SSAValue) && return renumber_ssa(stmt, ssanums, new_ssa, used_ssa)
return ssamap(val->renumber_ssa(val, ssanums, new_ssa, used_ssa), stmt)
function renumber_ssa!(@nospecialize(stmt), ssanums::Vector{SSAValue}, new_ssa::Bool=false)
isa(stmt, SSAValue) && return renumber_ssa(stmt, ssanums, new_ssa)
return ssamap(val->renumber_ssa(val, ssanums, new_ssa), stmt)
end

function make_ssa!(ci::CodeInfo, code::Vector{Any}, idx, slot, @nospecialize(typ))
Expand Down Expand Up @@ -428,8 +426,11 @@ function domsort_ssa!(ir::IRCode, domtree::DomTree)
end
end
result = InstructionStream(nstmts + ncritbreaks + nnewfallthroughs)
inst_rename = Vector{Any}(undef, length(ir.stmts) + length(ir.new_nodes))
for i = 1:length(ir.new_nodes)
inst_rename = Vector{SSAValue}(undef, length(ir.stmts) + length(ir.new_nodes))
@inbounds for i = 1:length(ir.stmts)
inst_rename[i] = SSAValue(-1)
end
@inbounds for i = 1:length(ir.new_nodes)
inst_rename[i + length(ir.stmts)] = SSAValue(i + length(result))
end
bb_start_off = 0
Expand Down Expand Up @@ -802,7 +803,7 @@ function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, narg
# Convert into IRCode form
nstmts = length(ir.stmts)
new_code = Vector{Any}(undef, nstmts)
ssavalmap = Any[SSAValue(-1) for _ in 0:length(ci.ssavaluetypes)]
ssavalmap = fill(SSAValue(-1), length(ci.ssavaluetypes) + 1)
result_types = Any[Any for _ in 1:nstmts]
# Detect statement positions for assignments and construct array
for (bb, idx) in bbidxiter(ir)
Expand Down

2 comments on commit a0a68a5

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.