Skip to content

Commit

Permalink
8292602: ZGC: C2 late barrier analysis uses invalid dominator informa…
Browse files Browse the repository at this point in the history
…tion

Backport-of: eec992c6b0ac77d08478d852a80c9470418d925d
  • Loading branch information
GoeLin committed Jan 2, 2023
1 parent 4be52ee commit 330105c
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 0 deletions.
60 changes: 60 additions & 0 deletions src/hotspot/share/opto/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,48 @@ void PhaseCFG::insert_goto_at(uint block_no, uint succ_no) {
block->_freq = freq;
// add new basic block to basic block list
add_block_at(block_no + 1, block);
// Update dominator tree information of the new goto block.
block->_idom = in;
block->_dom_depth = in->_dom_depth + 1;
if (out->_idom != in) {
// The successor block was not immediately dominated by the predecessor
// block, so there is no dominator subtree to update.
return;
}
// Update immediate dominator of the successor block.
out->_idom = block;
// Increment the dominator tree depth of the goto block's descendants. These
// are found by a depth-first search starting from the successor block. Two
// domination properties guarantee that only descendant blocks are visited:
// 1) all dominators of a block b must appear in any path from the root to b;
// 2) if a block b does not dominate another block b', b cannot dominate any
// block reachable from b' either.
// The exploration uses header indices as block identifiers, since
// Block::_pre_order might not be unique in the context of this function.
ResourceMark rm;
VectorSet descendants;
descendants.set(block->head()->_idx); // The goto block is a descendant of itself.
Block_List worklist;
worklist.push(out); // Start exploring from the successor block.
while (worklist.size() > 0) {
Block* b = worklist.pop();
// The immediate dominator of b is a descendant, hence b is also a
// descendant. Even though all predecessors of b might not have been visited
// yet, we know that all dominators of b have been already visited (since
// they must appear in any path from the goto block to b).
descendants.set(b->head()->_idx);
b->_dom_depth++;
for (uint i = 0; i < b->_num_succs; i++) {
Block* s = b->_succs[i];
if (s != get_root_block() &&
!descendants.test(s->head()->_idx) &&
// Do not search below non-descendant successors, since any block
// reachable from them cannot be descendant either.
descendants.test(s->_idom->head()->_idx)) {
worklist.push(s);
}
}
}
}

// Does this block end in a multiway branch that cannot have the default case
Expand Down Expand Up @@ -1275,6 +1317,23 @@ void PhaseCFG::verify_memory_writer_placement(const Block* b, const Node* n) con
assert(found, "block b is not in n's home loop or an ancestor of it");
}

void PhaseCFG::verify_dominator_tree() const {
for (uint i = 0; i < number_of_blocks(); i++) {
Block* block = get_block(i);
assert(block->_dom_depth <= number_of_blocks(), "unexpected dominator tree depth");
if (block == get_root_block()) {
assert(block->_dom_depth == 1, "unexpected root dominator tree depth");
// The root block does not have an immediate dominator, stop checking.
continue;
}
assert(block->_idom != nullptr, "non-root blocks must have immediate dominators");
assert(block->_dom_depth == block->_idom->_dom_depth + 1,
"the dominator tree depth of a node must succeed that of its immediate dominator");
assert(block->num_preds() > 2 || block->_idom == get_block_for_node(block->pred(1)),
"the immediate dominator of a single-predecessor block must be the predecessor");
}
}

void PhaseCFG::verify() const {
// Verify sane CFG
for (uint i = 0; i < number_of_blocks(); i++) {
Expand Down Expand Up @@ -1344,6 +1403,7 @@ void PhaseCFG::verify() const {
assert(block->_num_succs == 2, "Conditional branch must have two targets");
}
}
verify_dominator_tree();
}
#endif // ASSERT

Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/block.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,8 @@ class PhaseCFG : public Phase {
// Check that block b is in the home loop (or an ancestor) of n, if n is a
// memory writer.
void verify_memory_writer_placement(const Block* b, const Node* n) const NOT_DEBUG_RETURN;
// Check local dominator tree invariants.
void verify_dominator_tree() const NOT_DEBUG_RETURN;
void verify() const NOT_DEBUG_RETURN;
};

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2762,6 +2762,7 @@ void Compile::Code_Gen() {
}
cfg.fixup_flow();
cfg.remove_unreachable_blocks();
cfg.verify_dominator_tree();
}

// Apply peephole optimizations
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/opto/idealGraphPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ void IdealGraphPrinter::visit_node(Node *n, bool edges, VectorSet* temp_set) {
print_prop("block", C->cfg()->get_block(0)->_pre_order);
} else {
print_prop("block", block->_pre_order);
if (node == block->head()) {
if (block->_idom != NULL) {
print_prop("idom", block->_idom->_pre_order);
}
print_prop("dom_depth", block->_dom_depth);
}
// Print estimated execution frequency, normalized within a [0,1] range.
buffer[0] = 0;
stringStream freq(buffer, sizeof(buffer) - 1);
Expand Down

1 comment on commit 330105c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.