Skip to content

Commit

Permalink
Fix handling of virtual exit node in PostDomTree (#53739)
Browse files Browse the repository at this point in the history
This is an alternative to #53642

The `dom_edges()` for an exit block in the CFG are empty when computing
the PostDomTree so the loop below this may not actually run. In that
case, the right semidominator is the ancestor from the DFSTree, which is
the "virtual" -1 block.

This resolves half of the issue in
#53613:
```julia
julia> let code = Any[
               # block 1
               GotoIfNot(Argument(2), 3),
               # block 2
               ReturnNode(Argument(3)),
               # block 3 (we should visit this block)
               Expr(:call, throw, "potential throw"),
               ReturnNode(), # unreachable
           ]
           ir = make_ircode(code; slottypes=Any[Any,Bool,Bool])
           visited = BitSet()
           @test !Core.Compiler.visit_conditional_successors(CC.LazyPostDomtree(ir), ir, #=bb=#1) do succ::Int
               push!(visited, succ)
               return false
           end
           @test 2 ∈ visited
           @test 3 ∈ visited
       end
Test Passed
```

This needs some tests (esp. since I don't think we have any DomTree
tests at all right now), but otherwise should be good to go.
  • Loading branch information
topolarity committed Mar 19, 2024
1 parent 8f76c69 commit 2775c9a
Showing 1 changed file with 1 addition and 4 deletions.
5 changes: 1 addition & 4 deletions base/compiler/ssair/domtree.jl
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,7 @@ function SNCA!(domtree::GenericDomTree{IsPostDom}, blocks::Vector{BasicBlock}, m
ancestors = copy(D.to_parent_pre)
relevant_blocks = IsPostDom ? (1:max_pre) : (2:max_pre)
for w::PreNumber in reverse(relevant_blocks)
# LLVM initializes this to the parent, the paper initializes this to
# `w`, but it doesn't really matter (the parent is a predecessor, so at
# worst we'll discover it below). Save a memory reference here.
semi_w = typemax(PreNumber)
semi_w = ancestors[w]
last_linked = PreNumber(w + 1)
for v dom_edges(domtree, blocks, D.from_pre[w])
# For the purpose of the domtree, ignore virtual predecessors into
Expand Down

4 comments on commit 2775c9a

@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 package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

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

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

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Please sign in to comment.