Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8295867: TestVerifyGraphEdges.java fails with exit code -1073741571 when using AlwaysIncrementalInline #11065

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4238,8 +4238,12 @@ bool Compile::needs_clinit_barrier(ciInstanceKlass* holder, ciMethod* accessing_
void Compile::verify_graph_edges(bool no_dead_code) {
if (VerifyGraphEdges) {
Unique_Node_List visited;
// Allocate stack of size C->live_nodes()/16 to avoid frequent realloc
uint stack_size = live_nodes() >> 4;
Node_List nstack(MAX2(stack_size, (uint)OptoNodeListSize));
Copy link
Member

Choose a reason for hiding this comment

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

As you only need the stack in verify_edges(), I suggest to move these lines directly into the method verify_edges().

Copy link
Contributor Author

@vnkozlov vnkozlov Nov 10, 2022

Choose a reason for hiding this comment

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

I need live_nodes() value or stack_size or C to pass for creating list inside method.
I decided to move renamed verify_bidirectional_edges() method to Compile class to get these values inside the method.
It does not need to be in Node class after I removed recursion.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I've missed that before. Moving it to Compile is a good idea to apply that!


// Call recursive graph walk to check edges
_root->verify_edges(visited);
Node::verify_edges(_root, visited, nstack);
if (no_dead_code) {
// Now make sure that no visited node is used by an unvisited node.
bool dead_nodes = false;
Expand Down
76 changes: 41 additions & 35 deletions src/hotspot/share/opto/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2683,46 +2683,52 @@ void Node::dump_comp(const char* suffix, outputStream *st) const {
// For each input edge to a node (ie - for each Use-Def edge), verify that
// there is a corresponding Def-Use edge.
//------------------------------verify_edges-----------------------------------
void Node::verify_edges(Unique_Node_List &visited) {
uint i, j, idx;
int cnt;
Node *n;
void Node::verify_edges(Node* root, Unique_Node_List &visited, Node_List &nstack) {
nstack.push(root);

// Recursive termination test
if (visited.member(this)) return;
visited.push(this);

// Walk over all input edges, checking for correspondence
for( i = 0; i < len(); i++ ) {
n = in(i);
if (n != NULL && !n->is_top()) {
// Count instances of (Node *)this
cnt = 0;
for (idx = 0; idx < n->_outcnt; idx++ ) {
if (n->_out[idx] == (Node *)this) cnt++;
}
assert( cnt > 0,"Failed to find Def-Use edge." );
// Check for duplicate edges
// walk the input array downcounting the input edges to n
for( j = 0; j < len(); j++ ) {
if( in(j) == n ) cnt--;
while (nstack.size() > 0) {
Node* next = nstack.pop();
if (visited.member(next)) {
continue;
}
visited.push(next);

// Walk over all input edges, checking for correspondence
uint length = next->len();
for (uint i = 0; i < length; i++) {
Node* n = next->in(i);
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to rename n to input to make it easier to see if it is the current node or an input to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to in

if (n != NULL && !visited.member(n)) {
nstack.push(n); // Put it on stack
}
assert( cnt == 0,"Mismatched edge count.");
} else if (n == NULL) {
assert(i >= req() || i == 0 || is_Region() || is_Phi() || is_ArrayCopy() || (is_Unlock() && i == req()-1)
|| (is_MemBar() && i == 5), // the precedence edge to a membar can be removed during macro node expansion
if (n != NULL && !n->is_top()) {
// Count instances of `next`
int cnt = 0;
for (uint idx = 0; idx < n->_outcnt; idx++) {
if (n->_out[idx] == next) {
cnt++;
}
}
assert(cnt > 0, "Failed to find Def-Use edge.");
// Check for duplicate edges
// walk the input array downcounting the input edges to n
for(uint j = 0; j < length; j++) {
vnkozlov marked this conversation as resolved.
Show resolved Hide resolved
if (next->in(j) == n) {
cnt--;
}
}
assert(cnt == 0, "Mismatched edge count.");
} else if (n == NULL) {
assert(i >= next->req() || i == 0 ||
next->is_Region() || next->is_Phi() || next->is_ArrayCopy() ||
(next->is_Unlock() && i == (next->req() - 1)) ||
(next->is_MemBar() && i == 5), // the precedence edge to a membar can be removed during macro node expansion
"only region, phi, arraycopy, unlock or membar nodes have null data edges");
} else {
assert(n->is_top(), "sanity");
// Nothing to check.
} else {
assert(n->is_top(), "sanity");
// Nothing to check.
}
}
}
// Recursive walk over all input edges
for( i = 0; i < len(); i++ ) {
n = in(i);
if( n != NULL )
in(i)->verify_edges(visited);
}
}

// Verify all nodes if verify_depth is negative
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ class Node {
// Print compact per-node info
virtual void dump_compact_spec(outputStream *st) const { dump_spec(st); }

void verify_edges(Unique_Node_List &visited); // Verify bi-directional edges
static void verify_edges(Node* root, Unique_Node_List &visited, Node_List &nstack); // Verify bi-directional edges
vnkozlov marked this conversation as resolved.
Show resolved Hide resolved
static void verify(int verify_depth, VectorSet& visited, Node_List& worklist);

// This call defines a class-unique string used to identify class instances
Expand Down