From eb6f45745d0ded9da61fa52f948f9b855bc57958 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Sat, 27 Jan 2024 00:21:18 +0100 Subject: [PATCH 1/3] 8324790: ifnode::fold_compares_helper cleanup --- src/hotspot/share/opto/ifnode.cpp | 185 ++++++++++++++---------------- 1 file changed, 87 insertions(+), 98 deletions(-) diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 6ae62b24b3c8d..a96a0a289495d 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -624,64 +624,60 @@ Node* IfNode::up_one_dom(Node *curr, bool linear_only) { const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj) { assert(if_proj && (if_proj->Opcode() == Op_IfTrue || if_proj->Opcode() == Op_IfFalse), "expecting an if projection"); - if (if_proj->in(0) && if_proj->in(0)->is_If()) { - IfNode* iff = if_proj->in(0)->as_If(); - if (iff->in(1) && iff->in(1)->is_Bool()) { - BoolNode* bol = iff->in(1)->as_Bool(); - if (bol->in(1) && bol->in(1)->is_Cmp()) { - const CmpNode* cmp = bol->in(1)->as_Cmp(); - if (cmp->in(1) == val) { - const TypeInt* cmp2_t = gvn->type(cmp->in(2))->isa_int(); - if (cmp2_t != nullptr) { - jint lo = cmp2_t->_lo; - jint hi = cmp2_t->_hi; - BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); - switch (msk) { - case BoolTest::ne: { - // If val is compared to its lower or upper bound, we can narrow the type - const TypeInt* val_t = gvn->type(val)->isa_int(); - if (val_t != nullptr && !val_t->singleton() && cmp2_t->is_con()) { - if (val_t->_lo == lo) { - return TypeInt::make(val_t->_lo + 1, val_t->_hi, val_t->_widen); - } else if (val_t->_hi == hi) { - return TypeInt::make(val_t->_lo, val_t->_hi - 1, val_t->_widen); - } - } - // Can't refine type - return nullptr; - } - case BoolTest::eq: - return cmp2_t; - case BoolTest::lt: - lo = TypeInt::INT->_lo; - if (hi != min_jint) { - hi = hi - 1; - } - break; - case BoolTest::le: - lo = TypeInt::INT->_lo; - break; - case BoolTest::gt: - if (lo != max_jint) { - lo = lo + 1; - } - hi = TypeInt::INT->_hi; - break; - case BoolTest::ge: - // lo unchanged - hi = TypeInt::INT->_hi; - break; - default: - break; - } - const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); - return rtn_t; - } - } + IfNode* iff = if_proj->in(0)->as_If(); + BoolNode* bol = iff->in(1)->isa_Bool(); + if (bol == nullptr) { + return nullptr; + } + const CmpNode* cmp = bol->in(1)->as_Cmp(); + const TypeInt* cmp2_t = gvn->type(cmp->in(2))->isa_int(); + if (cmp->in(1) != val || cmp2_t == nullptr) { + return nullptr; + } + + jint lo = cmp2_t->_lo; + jint hi = cmp2_t->_hi; + BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); + switch (msk) { + case BoolTest::ne: { + // If val is compared to its lower or upper bound, we can narrow the type + const TypeInt* val_t = gvn->type(val)->isa_int(); + if (val_t != nullptr && !val_t->singleton() && cmp2_t->is_con()) { + if (val_t->_lo == lo) { + return TypeInt::make(val_t->_lo + 1, val_t->_hi, val_t->_widen); + } else if (val_t->_hi == hi) { + return TypeInt::make(val_t->_lo, val_t->_hi - 1, val_t->_widen); } } + // Can't refine type + return nullptr; } - return nullptr; + case BoolTest::eq: + return cmp2_t; + case BoolTest::lt: + lo = TypeInt::INT->_lo; + if (hi != min_jint) { + hi = hi - 1; + } + break; + case BoolTest::le: + lo = TypeInt::INT->_lo; + break; + case BoolTest::gt: + if (lo != max_jint) { + lo = lo + 1; + } + hi = TypeInt::INT->_hi; + break; + case BoolTest::ge: + // lo unchanged + hi = TypeInt::INT->_hi; + break; + default: + break; + } + const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); + return rtn_t; } //------------------------------fold_compares---------------------------- @@ -1015,7 +1011,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f const TypeInt* type2 = filtered_int_type(igvn, n, fail); if (type2 != nullptr) { failtype = failtype->join(type2)->is_int(); - if (failtype->_lo > failtype->_hi) { + if (failtype->empty()) { // previous if determines the result of this if so // replace Bool with constant igvn->replace_input_of(this, 1, igvn->intcon(success->_con)); @@ -1023,55 +1019,48 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f } } } - lo = nullptr; - hi = nullptr; + return false; + } + + Node* hook = new Node(lo); // Add a use to lo to prevent him from dying + // Merge the two compares into a single unsigned compare by building (CmpU (n - lo) (hi - lo)) + Node* adjusted_val = igvn->transform(new SubINode(n, lo)); + if (adjusted_lim == nullptr) { + adjusted_lim = igvn->transform(new SubINode(hi, lo)); } + hook->destruct(igvn); - if (lo && hi) { - Node* hook = new Node(1); - hook->init_req(0, lo); // Add a use to lo to prevent him from dying - // Merge the two compares into a single unsigned compare by building (CmpU (n - lo) (hi - lo)) - Node* adjusted_val = igvn->transform(new SubINode(n, lo)); - if (adjusted_lim == nullptr) { - adjusted_lim = igvn->transform(new SubINode(hi, lo)); + if (igvn->type(adjusted_lim)->is_int()->_lo < 0 && + !igvn->C->post_loop_opts_phase()) { + // If range check elimination applies to this comparison, it includes code to protect from overflows that may + // cause the main loop to be skipped entirely. Delay this transformation. + // Example: + // for (int i = 0; i < limit; i++) { + // if (i < max_jint && i > min_jint) {... + // } + // Comparisons folded as: + // i - min_jint - 1 outcnt() == 0) { + igvn->remove_dead_node(adjusted_val); } - hook->destruct(igvn); - - int lo = igvn->type(adjusted_lim)->is_int()->_lo; - if (lo < 0) { - // If range check elimination applies to this comparison, it includes code to protect from overflows that may - // cause the main loop to be skipped entirely. Delay this transformation. - // Example: - // for (int i = 0; i < limit; i++) { - // if (i < max_jint && i > min_jint) {... - // } - // Comparisons folded as: - // i - min_jint - 1 C->post_loop_opts_phase()) { - if (adjusted_val->outcnt() == 0) { - igvn->remove_dead_node(adjusted_val); - } - if (adjusted_lim->outcnt() == 0) { - igvn->remove_dead_node(adjusted_lim); - } - igvn->C->record_for_post_loop_opts_igvn(this); - return false; - } + if (adjusted_lim->outcnt() == 0) { + igvn->remove_dead_node(adjusted_lim); } + igvn->C->record_for_post_loop_opts_igvn(this); + return false; + } - Node* newcmp = igvn->transform(new CmpUNode(adjusted_val, adjusted_lim)); - Node* newbool = igvn->transform(new BoolNode(newcmp, cond)); + Node* newcmp = igvn->transform(new CmpUNode(adjusted_val, adjusted_lim)); + Node* newbool = igvn->transform(new BoolNode(newcmp, cond)); - igvn->replace_input_of(dom_iff, 1, igvn->intcon(proj->_con)); - igvn->replace_input_of(this, 1, newbool); + igvn->replace_input_of(dom_iff, 1, igvn->intcon(proj->_con)); + igvn->replace_input_of(this, 1, newbool); - return true; - } - return false; + return true; } // Merge the branches that trap for this If and the dominating If into From 19e1fa81c5521ea858fcba64ac993081285023ca Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Sat, 17 Feb 2024 00:54:38 +0100 Subject: [PATCH 2/3] Undo changes in filtered_int_type() --- src/hotspot/share/opto/ifnode.cpp | 106 ++++++++++++++++-------------- 1 file changed, 55 insertions(+), 51 deletions(-) diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index a96a0a289495d..1568d5da186f2 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -624,60 +624,64 @@ Node* IfNode::up_one_dom(Node *curr, bool linear_only) { const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj) { assert(if_proj && (if_proj->Opcode() == Op_IfTrue || if_proj->Opcode() == Op_IfFalse), "expecting an if projection"); - IfNode* iff = if_proj->in(0)->as_If(); - BoolNode* bol = iff->in(1)->isa_Bool(); - if (bol == nullptr) { - return nullptr; - } - const CmpNode* cmp = bol->in(1)->as_Cmp(); - const TypeInt* cmp2_t = gvn->type(cmp->in(2))->isa_int(); - if (cmp->in(1) != val || cmp2_t == nullptr) { - return nullptr; - } - - jint lo = cmp2_t->_lo; - jint hi = cmp2_t->_hi; - BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); - switch (msk) { - case BoolTest::ne: { - // If val is compared to its lower or upper bound, we can narrow the type - const TypeInt* val_t = gvn->type(val)->isa_int(); - if (val_t != nullptr && !val_t->singleton() && cmp2_t->is_con()) { - if (val_t->_lo == lo) { - return TypeInt::make(val_t->_lo + 1, val_t->_hi, val_t->_widen); - } else if (val_t->_hi == hi) { - return TypeInt::make(val_t->_lo, val_t->_hi - 1, val_t->_widen); + if (if_proj->in(0) && if_proj->in(0)->is_If()) { + IfNode* iff = if_proj->in(0)->as_If(); + if (iff->in(1) && iff->in(1)->is_Bool()) { + BoolNode* bol = iff->in(1)->as_Bool(); + if (bol->in(1) && bol->in(1)->is_Cmp()) { + const CmpNode* cmp = bol->in(1)->as_Cmp(); + if (cmp->in(1) == val) { + const TypeInt* cmp2_t = gvn->type(cmp->in(2))->isa_int(); + if (cmp2_t != nullptr) { + jint lo = cmp2_t->_lo; + jint hi = cmp2_t->_hi; + BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); + switch (msk) { + case BoolTest::ne: { + // If val is compared to its lower or upper bound, we can narrow the type + const TypeInt* val_t = gvn->type(val)->isa_int(); + if (val_t != nullptr && !val_t->singleton() && cmp2_t->is_con()) { + if (val_t->_lo == lo) { + return TypeInt::make(val_t->_lo + 1, val_t->_hi, val_t->_widen); + } else if (val_t->_hi == hi) { + return TypeInt::make(val_t->_lo, val_t->_hi - 1, val_t->_widen); + } + } + // Can't refine type + return nullptr; + } + case BoolTest::eq: + return cmp2_t; + case BoolTest::lt: + lo = TypeInt::INT->_lo; + if (hi != min_jint) { + hi = hi - 1; + } + break; + case BoolTest::le: + lo = TypeInt::INT->_lo; + break; + case BoolTest::gt: + if (lo != max_jint) { + lo = lo + 1; + } + hi = TypeInt::INT->_hi; + break; + case BoolTest::ge: + // lo unchanged + hi = TypeInt::INT->_hi; + break; + default: + break; + } + const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); + return rtn_t; + } + } } } - // Can't refine type - return nullptr; } - case BoolTest::eq: - return cmp2_t; - case BoolTest::lt: - lo = TypeInt::INT->_lo; - if (hi != min_jint) { - hi = hi - 1; - } - break; - case BoolTest::le: - lo = TypeInt::INT->_lo; - break; - case BoolTest::gt: - if (lo != max_jint) { - lo = lo + 1; - } - hi = TypeInt::INT->_hi; - break; - case BoolTest::ge: - // lo unchanged - hi = TypeInt::INT->_hi; - break; - default: - break; - } - const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); - return rtn_t; + return nullptr; } //------------------------------fold_compares---------------------------- From 341b5006a440378f07514b52d5bd1e0994b6d246 Mon Sep 17 00:00:00 2001 From: Joshua Cao Date: Tue, 27 Feb 2024 00:44:58 +0100 Subject: [PATCH 3/3] sanity check that hi and lo are not nullptr --- src/hotspot/share/opto/ifnode.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 1568d5da186f2..0dad48ffc73f7 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -1026,6 +1026,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f return false; } + assert(lo != nullptr && hi != nullptr, "sanity"); Node* hook = new Node(lo); // Add a use to lo to prevent him from dying // Merge the two compares into a single unsigned compare by building (CmpU (n - lo) (hi - lo)) Node* adjusted_val = igvn->transform(new SubINode(n, lo));