From 11aece21f4eb5b18af357b265bc27b80bcdbfbcb Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Fri, 9 Dec 2022 07:11:57 +0000 Subject: [PATCH] 8257197: Add additional verification code to PhaseCCP Reviewed-by: chagedorn, kvn, thartmann --- src/hotspot/share/opto/phaseX.cpp | 48 +++++++++++++++++++++++++++++++ src/hotspot/share/opto/phaseX.hpp | 4 +++ 2 files changed, 52 insertions(+) diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp index d5c861e14aa..2c260ebf4f9 100644 --- a/src/hotspot/share/opto/phaseX.cpp +++ b/src/hotspot/share/opto/phaseX.cpp @@ -1782,6 +1782,7 @@ void PhaseCCP::analyze() { // Push root onto worklist Unique_Node_List worklist; worklist.push(C->root()); + DEBUG_ONLY(Unique_Node_List worklist_verify;) assert(_root_and_safepoints.size() == 0, "must be empty (unused)"); _root_and_safepoints.push(C->root()); @@ -1790,6 +1791,7 @@ void PhaseCCP::analyze() { // This loop is the meat of CCP. while (worklist.size() != 0) { Node* n = fetch_next_node(worklist); + DEBUG_ONLY(worklist_verify.push(n);) if (n->is_SafePoint()) { // Make sure safepoints are processed by PhaseCCP::transform even if they are // not reachable from the bottom. Otherwise, infinite loops would be removed. @@ -1803,8 +1805,54 @@ void PhaseCCP::analyze() { push_child_nodes_to_worklist(worklist, n); } } + DEBUG_ONLY(verify_analyze(worklist_verify);) } +#ifdef ASSERT +// For every node n on verify list, check if type(n) == n->Value() +// We have a list of exceptions, see comments in code. +void PhaseCCP::verify_analyze(Unique_Node_List& worklist_verify) { + bool failure = false; + while (worklist_verify.size()) { + Node* n = worklist_verify.pop(); + const Type* told = type(n); + const Type* tnew = n->Value(this); + if (told != tnew) { + // Check special cases that are ok + if (told->isa_integer(tnew->basic_type()) != nullptr) { // both either int or long + const TypeInteger* t0 = told->is_integer(tnew->basic_type()); + const TypeInteger* t1 = tnew->is_integer(tnew->basic_type()); + if (t0->lo_as_long() == t1->lo_as_long() && + t0->hi_as_long() == t1->hi_as_long()) { + continue; // ignore integer widen + } + } + if (n->is_Load()) { + // MemNode::can_see_stored_value looks up through many memory nodes, + // which means we would need to notify modifications from far up in + // the inputs all the way down to the LoadNode. We don't do that. + continue; + } + tty->cr(); + tty->print_cr("Missed optimization (PhaseCCP):"); + n->dump_bfs(1, 0, ""); + tty->print_cr("Current type:"); + told->dump_on(tty); + tty->cr(); + tty->print_cr("Optimized type:"); + tnew->dump_on(tty); + tty->cr(); + failure = true; + } + } + // If we get this assert, check why the reported nodes were not processed again in CCP. + // We should either make sure that these nodes are properly added back to the CCP worklist + // in PhaseCCP::push_child_nodes_to_worklist() to update their type or add an exception + // in the verification code above if that is not possible for some reason (like Load nodes). + assert(!failure, "Missed optimization opportunity in PhaseCCP"); +} +#endif + // Fetch next node from worklist to be examined in this iteration. Node* PhaseCCP::fetch_next_node(Unique_Node_List& worklist) { if (StressCCP) { diff --git a/src/hotspot/share/opto/phaseX.hpp b/src/hotspot/share/opto/phaseX.hpp index c51aed5b762..883dc92e7ef 100644 --- a/src/hotspot/share/opto/phaseX.hpp +++ b/src/hotspot/share/opto/phaseX.hpp @@ -603,6 +603,10 @@ class PhaseCCP : public PhaseIterGVN { // Worklist algorithm identifies constants void analyze(); +#ifdef ASSERT + // For every node n on verify list, check if type(n) == n->Value() + void verify_analyze(Unique_Node_List& worklist_verify); +#endif // Recursive traversal of program. Used analysis to modify program. virtual Node *transform( Node *n ); // Do any transformation after analysis