Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

8289069: Very slow C1 arraycopy jcstress tests after JDK-8279886 #72

Closed
wants to merge 2 commits into from
Closed

8289069: Very slow C1 arraycopy jcstress tests after JDK-8279886 #72

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f20dec
Size bitmaps appropriately
veresov Jun 25, 2022
8abfae6
Fix a typo
veresov Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 32 additions & 22 deletions src/hotspot/share/c1/c1_GraphBuilder.cpp
View file
Open in desktop
Expand Up @@ -66,7 +66,9 @@ class BlockListBuilder {
GrowableArray<ResourceBitMap> _loop_map; // caches the information if a block is contained in a loop
int _next_loop_index; // next free loop number
int _next_block_number; // for reverse postorder numbering of blocks
int _block_id_start;

int bit_number(int block_id) const { return block_id - _block_id_start; }
// accessors
Compilation* compilation() const { return _compilation; }
IRScope* scope() const { return _scope; }
Expand Down Expand Up @@ -122,6 +124,7 @@ BlockListBuilder::BlockListBuilder(Compilation* compilation, IRScope* scope, int
, _loop_map() // size not known yet
, _next_loop_index(0)
, _next_block_number(0)
, _block_id_start(0)
{
set_entries(osr_bci);
set_leaders();
Expand Down Expand Up @@ -384,11 +387,12 @@ void BlockListBuilder::set_leaders() {
void BlockListBuilder::mark_loops() {
ResourceMark rm;

_active.initialize(BlockBegin::number_of_blocks());
_visited.initialize(BlockBegin::number_of_blocks());
_loop_map = GrowableArray<ResourceBitMap>(BlockBegin::number_of_blocks(), BlockBegin::number_of_blocks(), ResourceBitMap());
for (int i = 0; i < BlockBegin::number_of_blocks(); i++) {
_loop_map.at(i).initialize(BlockBegin::number_of_blocks());
const int number_of_blocks = _blocks.length();
_active.initialize(number_of_blocks);
_visited.initialize(number_of_blocks);
_loop_map = GrowableArray<ResourceBitMap>(number_of_blocks, number_of_blocks, ResourceBitMap());
for (int i = 0; i < number_of_blocks; i++) {
_loop_map.at(i).initialize(number_of_blocks);
}
_next_loop_index = 0;
_next_block_number = _blocks.length();
Expand All @@ -401,12 +405,14 @@ void BlockListBuilder::mark_loops() {
// - The bit is then propagated for all the blocks in the loop after we exit them (post-order). There could be multiple bits
// of course in case of nested loops.
// - When we exit the loop header we remove that single bit and assign the real loop state for it.
// - Now, the tricky part here is how we detect irriducible loops. In the algorithm above the loop state bits
// - Now, the tricky part here is how we detect irreducible loops. In the algorithm above the loop state bits
// are propagated to the predecessors. If we encounter an irreducible loop (a loop with multiple heads) we would see
// a node with some loop bit set that would then propagate back and be never cleared because we would
// never go back through the original loop header. Therefore if there are any irreducible loops the bits in the states
// for these loops are going to propagate back to the root.
BitMap& loop_state = mark_loops(_bci2block->at(0), false);
BlockBegin* start = _bci2block->at(0);
Copy link
Member

Choose a reason for hiding this comment

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

There is a typo in line 408, that you might want to fix as well irriducible -> irreducible.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. Thanks!

_block_id_start = start->block_id();
BitMap& loop_state = mark_loops(start, false);
if (!loop_state.is_empty()) {
compilation()->set_has_irreducible_loops(true);
}
Expand All @@ -419,6 +425,8 @@ void BlockListBuilder::mark_loops() {
}

void BlockListBuilder::make_loop_header(BlockBegin* block) {
int block_id = block->block_id();
int block_bit = bit_number(block_id);
if (block->is_set(BlockBegin::exception_entry_flag)) {
// exception edges may look like loops but don't mark them as such
// since it screws up block ordering.
Expand All @@ -427,43 +435,45 @@ void BlockListBuilder::make_loop_header(BlockBegin* block) {
if (!block->is_set(BlockBegin::parser_loop_header_flag)) {
block->set(BlockBegin::parser_loop_header_flag);

assert(_loop_map.at(block->block_id()).is_empty(), "must not be set yet");
assert(0 <= _next_loop_index && _next_loop_index < BlockBegin::number_of_blocks(), "_next_loop_index is too large");
_loop_map.at(block->block_id()).set_bit(_next_loop_index++);
assert(_loop_map.at(block_bit).is_empty(), "must not be set yet");
assert(0 <= _next_loop_index && _next_loop_index < _loop_map.length(), "_next_loop_index is too large");
_loop_map.at(block_bit).set_bit(_next_loop_index++);
} else {
// block already marked as loop header
assert(_loop_map.at(block->block_id()).count_one_bits() == 1, "exactly one bit must be set");
assert(_loop_map.at(block_bit).count_one_bits() == 1, "exactly one bit must be set");
}
}

BitMap& BlockListBuilder::mark_loops(BlockBegin* block, bool in_subroutine) {
int block_id = block->block_id();
if (_visited.at(block_id)) {
if (_active.at(block_id)) {
int block_bit = bit_number(block_id);
if (_visited.at(block_bit)) {
if (_active.at(block_bit)) {
// reached block via backward branch
make_loop_header(block);
}
// return cached loop information for this block
return _loop_map.at(block_id);
return _loop_map.at(block_bit);
}

if (block->is_set(BlockBegin::subroutine_entry_flag)) {
in_subroutine = true;
}

// set active and visited bits before successors are processed
_visited.set_bit(block_id);
_active.set_bit(block_id);
_visited.set_bit(block_bit);
_active.set_bit(block_bit);

ResourceMark rm;
ResourceBitMap loop_state(BlockBegin::number_of_blocks());
ResourceBitMap loop_state(_loop_map.length());
for (int i = number_of_successors(block) - 1; i >= 0; i--) {
BlockBegin* sux = successor_at(block, i);
// recursively process all successors
loop_state.set_union(mark_loops(successor_at(block, i), in_subroutine));
loop_state.set_union(mark_loops(sux, in_subroutine));
}

// clear active-bit after all successors are processed
_active.clear_bit(block_id);
_active.clear_bit(block_bit);

// reverse-post-order numbering of all blocks
block->set_depth_first_number(_next_block_number);
Expand All @@ -476,15 +486,15 @@ BitMap& BlockListBuilder::mark_loops(BlockBegin* block, bool in_subroutine) {
}

if (block->is_set(BlockBegin::parser_loop_header_flag)) {
BitMap& header_loop_state = _loop_map.at(block_id);
BitMap& header_loop_state = _loop_map.at(block_bit);
assert(header_loop_state.count_one_bits() == 1, "exactly one bit must be set");
// remove the bit with the loop number for the state (header is outside of the loop)
loop_state.set_difference(header_loop_state);
}

// cache and return loop information for this block
_loop_map.at(block_id).set_from(loop_state);
return _loop_map.at(block_id);
_loop_map.at(block_bit).set_from(loop_state);
return _loop_map.at(block_bit);
}

inline int BlockListBuilder::number_of_successors(BlockBegin* block)
Expand Down