Skip to content

Commit

Permalink
Merge pull request #33368 from yhls/yhls/fixrenaming
Browse files Browse the repository at this point in the history
fix bug related to block renaming for DCE
  • Loading branch information
vchuravy authored Sep 25, 2019
2 parents 4a2a59a + 55759af commit 9a1dbc0
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
4 changes: 2 additions & 2 deletions base/compiler/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1001,14 +1001,14 @@ end

function finish_current_bb!(compact, active_bb, old_result_idx=compact.result_idx, unreachable=false)
if compact.active_result_bb > length(compact.result_bbs)
#@assert compact.bb_rename[active_bb] == 0
#@assert compact.bb_rename[active_bb] == -1
return true
end
bb = compact.result_bbs[compact.active_result_bb]
# If this was the last statement in the BB and we decided to skip it, insert a
# dummy `nothing` node, to prevent changing the structure of the CFG
skipped = false
if !compact.cfg_transforms_enabled || active_bb == 0 || active_bb > length(compact.bb_rename_succ) || compact.bb_rename_succ[active_bb] != 0
if !compact.cfg_transforms_enabled || active_bb == 0 || active_bb > length(compact.bb_rename_succ) || compact.bb_rename_succ[active_bb] != -1
if compact.result_idx == first(bb.stmts)
length(compact.result) < old_result_idx && resize!(compact, old_result_idx)
if unreachable
Expand Down
31 changes: 31 additions & 0 deletions test/compiler/ssair.jl
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,34 @@ function f32579(x::Int, b::Bool)
end
@test f32579(0, true) === true
@test f32579(0, false) === false

# Test for bug caused by renaming blocks improperly, related to PR #32145
using Base.Meta
let ci = (Meta.@lower 1 + 1).args[1]
ci.code = [
# block 1
Core.Compiler.GotoIfNot(Expr(:boundscheck), 6),
# block 2
Expr(:call, GlobalRef(Base, :size), Core.Compiler.Argument(3)),
Core.Compiler.ReturnNode(),
# block 3
Core.PhiNode(),
Core.Compiler.ReturnNode(),
# block 4
Expr(:call,
GlobalRef(Main, :something),
GlobalRef(Main, :somethingelse)),
Core.Compiler.GotoIfNot(Core.SSAValue(6), 9),
# block 5
Core.Compiler.ReturnNode(Core.SSAValue(6)),
# block 6
Core.Compiler.ReturnNode(Core.SSAValue(6))
]
nstmts = length(ci.code)
ci.ssavaluetypes = nstmts
ci.codelocs = fill(Int32(1), nstmts)
ci.ssaflags = fill(Int32(0), nstmts)
ir = Core.Compiler.inflate_ir(ci)
ir = Core.Compiler.compact!(ir, true)
@test Core.Compiler.verify_ir(ir) == nothing
end

3 comments on commit 9a1dbc0

@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.

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`/home/nanosoldier/workdir/tmp8ospPu/julia -e 'VERSION >= v"0.7.0-DEV.3656" && using Pkg; Pkg.update()'`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@ararslan
Copy link
Member

Choose a reason for hiding this comment

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

Awesome. I don't know when I'll have time to look into that.

Please sign in to comment.