From 8ee73a7774e53744724ee28b5df000dbdab24ba0 Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Sat, 27 Feb 2021 01:37:39 +0530 Subject: [PATCH 1/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/cfgnode.hpp | 8 +- src/hotspot/share/opto/ifnode.cpp | 235 +++++++++++++----- .../jtreg/compiler/c2/TestJumpTable.java | 202 ++++++++++++++- 3 files changed, 379 insertions(+), 66 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index d0a22b78b6e54..272c946ea5727 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -288,7 +288,7 @@ class IfNode : public MultiBranchNode { private: // Helper methods for fold_compares - bool cmpi_folds(PhaseIterGVN* igvn, bool fold_ne = false); + bool cmpi_folds(PhaseIterGVN* igvn); bool is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn); bool has_shared_region(ProjNode* proj, ProjNode*& success, ProjNode*& fail); bool has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNode*& fail, PhaseIterGVN* igvn); @@ -299,7 +299,9 @@ class IfNode : public MultiBranchNode { bool is_side_effect_free_test(ProjNode* proj, PhaseIterGVN* igvn); void reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, PhaseIterGVN* igvn); ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call) const; - bool fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); + Node* get_base_comparing_value(Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val); + bool fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn); + bool fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); static bool is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* unc); protected: diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 482ccd63740e1..8a7381d403770 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -606,17 +606,19 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj 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(); + 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 != NULL) { jint lo = cmp2_t->_lo; jint hi = cmp2_t->_hi; + const TypeInt* val_t = gvn->type(val)->isa_int(); + bool is_unsigned = (cmp->Opcode() == Op_CmpU); BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); switch (msk) { case BoolTest::ne: { + assert(!is_unsigned, "unsigned comparison is not supported"); // 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 != NULL && !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); @@ -628,31 +630,49 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj return NULL; } case BoolTest::eq: + assert(!is_unsigned, "unsigned comparison is not supported"); return cmp2_t; case BoolTest::lt: - lo = TypeInt::INT->_lo; + if (is_unsigned && lo >= 0) { + // val u< cmp2 only passes for val >= 0 if cmp2 >= 0 + lo = 0; + } else { + lo = TypeInt::INT->_lo; + } if (hi != min_jint) { hi = hi - 1; } break; case BoolTest::le: - lo = TypeInt::INT->_lo; + if (is_unsigned && lo >= 0) { + // val u<= cmp2 only passes for val >= 0 if cmp2 >= 0 + lo = 0; + } else { + lo = TypeInt::INT->_lo; + } break; case BoolTest::gt: - if (lo != max_jint) { + if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) { + // val u> cmp2 passes for val < 0 + lo = TypeInt::INT->_lo; + } else if (lo != max_jint) { lo = lo + 1; } hi = TypeInt::INT->_hi; break; case BoolTest::ge: // lo unchanged + if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) { + // val u>= cmp2 passes for val < 0 + lo = TypeInt::INT->_lo; + } hi = TypeInt::INT->_hi; break; default: + assert(false, "should not reach here"); break; } - const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); - return rtn_t; + return TypeInt::make(lo, hi, cmp2_t->_widen); } } } @@ -700,16 +720,17 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj // // Is the comparison for this If suitable for folding? -bool IfNode::cmpi_folds(PhaseIterGVN* igvn, bool fold_ne) { +bool IfNode::cmpi_folds(PhaseIterGVN* igvn) { return in(1) != NULL && in(1)->is_Bool() && in(1)->in(1) != NULL && - in(1)->in(1)->Opcode() == Op_CmpI && + (in(1)->in(1)->Opcode() == Op_CmpI || + in(1)->in(1)->Opcode() == Op_CmpU) && in(1)->in(1)->in(2) != NULL && in(1)->in(1)->in(2) != igvn->C->top() && (in(1)->as_Bool()->_test.is_less() || in(1)->as_Bool()->_test.is_greater() || - (fold_ne && in(1)->as_Bool()->_test._test == BoolTest::ne)); + in(1)->as_Bool()->_test._test == BoolTest::ne); } // Is a dominating control suitable for folding with this if? @@ -719,10 +740,9 @@ bool IfNode::is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn) { ctrl->in(0) != NULL && ctrl->in(0)->Opcode() == Op_If && ctrl->in(0)->outcnt() == 2 && - ctrl->in(0)->as_If()->cmpi_folds(igvn, true) && - // Must compare same value + ctrl->in(0)->as_If()->cmpi_folds(igvn) && ctrl->in(0)->in(1)->in(1)->in(1) != NULL && - ctrl->in(0)->in(1)->in(1)->in(1) == in(1)->in(1)->in(1); + in(1)->in(1)->in(1) != NULL; } // Do this If and the dominating If share a region? @@ -837,8 +857,102 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod return false; } +// There might be an AddINode (marked with *) with a constant increment +// in-between the CmpNode(s) and the common value we compare. +// Check for the following cases. Then return common value compared +// if present and also save the constant values to be adjusted or +// subtracted due to the possible AddINode in-between. +// +// Variant 1 Variant 2 Variant 3 Variant 4 +// +// res_val res_val res_val res_val +// / \ / \ / \ / \ +// dom_cmp \ dom_val* \ / this_val* dom_val* this_val* +// this_cmp | \ / \ | \ +// dom_cmp \ dom_cmp \ dom_cmp \ +// this_cmp this_cmp this_cmp +Node* IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) { + Node* this_cmp = in(1)->in(1); + Node* dom_cmp = dom_if->in(1)->in(1); + assert(this_cmp->is_Cmp() != false && dom_cmp->is_Cmp() != false, "compare expected"); + + Node* res_val = NULL; + this_adj_val = 0; + dom_adj_val = 0; + Node* dom_val = dom_cmp->in(1); + Node* this_val = this_cmp->in(1); + if (this_val == dom_val) { + res_val = this_val; + } else if (this_val->is_Add() && this_val->in(1) && this_val->in(1) == dom_val) { + const TypeInt* val_t = igvn->type(this_val->in(2))->isa_int(); + if (val_t && val_t->is_con()) { + this_adj_val = val_t->_lo; + res_val = this_val->in(1); + } + } else if (dom_val->is_Add() && dom_val->in(1) && this_val == dom_val->in(1)) { + const TypeInt* val_t = igvn->type(dom_val->in(2))->isa_int(); + if (val_t && val_t->is_con()) { + dom_adj_val = val_t->_lo; + res_val = this_val; + } + } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) && dom_val->in(1) && this_val->in(1) == dom_val->in(1)) { + const TypeInt* domval_t = igvn->type(dom_val->in(2))->isa_int(); + const TypeInt* thisval_t = igvn->type(this_val->in(2))->isa_int(); + if (thisval_t && domval_t && thisval_t->is_con() && domval_t->is_con()) { + this_adj_val = thisval_t->_lo; + dom_adj_val = domval_t->_lo; + res_val = this_val->in(1); + } + } + + return res_val; +} + +// Check if dominating if determines the result of this if +bool IfNode::fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn) { + Node* this_cmp = in(1)->in(1); + Node* dom_if = proj->in(0)->as_If(); + Node* dom_cmp = dom_if->in(1)->in(1); + jint this_adj_val = 0; + jint dom_adj_val = 0; + Node* n = NULL; + + n = get_base_comparing_value(dom_if, igvn, this_adj_val, dom_adj_val); + if (n == NULL) { + // Not comparing same value + return false; + } + + const TypeInt* failtype = filtered_int_type(igvn, dom_cmp->in(1), proj); + if (failtype != NULL) { + if (dom_adj_val != 0) { + // To account for the AddINode, subtract the constant increment from the type + assert(dom_cmp->in(1)->is_Add() != false, "sanity"); + failtype = dom_cmp->in(1)->as_Add()->add_ring(failtype, TypeInt::make(-dom_adj_val))->is_int(); + } + for (int i = 0; i < 2; ++i) { + const TypeInt* type2 = filtered_int_type(igvn, this_cmp->in(1), proj_out(i)); + if (type2 != NULL) { + if (this_adj_val != 0) { + // To account for the AddINode, subtract the constant increment from the type + assert(this_cmp->in(1)->is_Add() != false, "sanity"); + type2 = this_cmp->in(1)->as_Add()->add_ring(type2, TypeInt::make(-this_adj_val))->is_int(); + } + const TypeInt* res_type = failtype->join(type2)->is_int(); + if (res_type->empty()) { + // Replace Bool with constant + igvn->_worklist.push(in(1)); + igvn->replace_input_of(this, 1, igvn->intcon(proj_out(1-i)->_con)); + return true; + } + } + } + } + return false; +} + // Check that the 2 CmpI can be folded into as single CmpU and proceed with the folding -bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) { +bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) { Node* this_cmp = in(1)->in(1); BoolNode* this_bool = in(1)->as_Bool(); IfNode* dom_iff = proj->in(0)->as_If(); @@ -847,13 +961,17 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f Node* hi = this_cmp->in(2); Node* n = this_cmp->in(1); ProjNode* otherproj = proj->other_if_proj(); - - const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj); - const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success); + assert(this_cmp->Opcode() == Op_CmpI && dom_iff->in(1)->in(1)->Opcode() == Op_CmpI, "Unexpected CmpNode"); BoolTest::mask lo_test = dom_bool->_test._test; BoolTest::mask hi_test = this_bool->_test._test; BoolTest::mask cond = hi_test; + if (lo_test == BoolTest::ne || hi_test == BoolTest::ne) { + return false; + } + + const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj); + const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success); // convert: // @@ -881,7 +999,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f // sets the lower bound if any. Node* adjusted_lim = NULL; if (lo_type != NULL && hi_type != NULL && hi_type->_lo > lo_type->_hi && - hi_type->_hi == max_jint && lo_type->_lo == min_jint && lo_test != BoolTest::ne) { + hi_type->_hi == max_jint && lo_type->_lo == min_jint) { assert((dom_bool->_test.is_less() && !proj->_con) || (dom_bool->_test.is_greater() && proj->_con), "incorrect test"); // this test was canonicalized @@ -926,7 +1044,7 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f return false; } } else if (lo_type != NULL && hi_type != NULL && lo_type->_lo > hi_type->_hi && - lo_type->_hi == max_jint && hi_type->_lo == min_jint && lo_test != BoolTest::ne) { + lo_type->_hi == max_jint && hi_type->_lo == min_jint) { // this_bool = < // dom_bool = < (proj = True) or dom_bool = >= (proj = False) @@ -984,20 +1102,6 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f return false; } } else { - const TypeInt* failtype = filtered_int_type(igvn, n, proj); - if (failtype != NULL) { - const TypeInt* type2 = filtered_int_type(igvn, n, fail); - if (type2 != NULL) { - failtype = failtype->join(type2)->is_int(); - if (failtype->_lo > failtype->_hi) { - // previous if determines the result of this if so - // replace Bool with constant - igvn->_worklist.push(in(1)); - igvn->replace_input_of(this, 1, igvn->intcon(success->_con)); - return true; - } - } - } lo = NULL; hi = NULL; } @@ -1263,40 +1367,55 @@ Node* IfNode::fold_compares(PhaseIterGVN* igvn) { if (cmpi_folds(igvn)) { Node* ctrl = in(0); - if (is_ctrl_folds(ctrl, igvn) && ctrl->outcnt() == 1) { - // A integer comparison immediately dominated by another integer - // comparison - ProjNode* success = NULL; - ProjNode* fail = NULL; - ProjNode* dom_cmp = ctrl->as_Proj(); - if (has_shared_region(dom_cmp, success, fail) && - // Next call modifies graph so must be last - fold_compares_helper(dom_cmp, success, fail, igvn)) { + Node* cmp = in(1)->in(1); + Node* val = cmp->in(1); + // An integer comparison immediately dominated by another integer comparison + if (is_ctrl_folds(ctrl, igvn)) { + ProjNode* proj = ctrl->as_Proj(); + if (fold_dominated_if(proj, igvn)) { return this; } - if (has_only_uncommon_traps(dom_cmp, success, fail, igvn) && - // Next call modifies graph so must be last - fold_compares_helper(dom_cmp, success, fail, igvn)) { - return merge_uncommon_traps(dom_cmp, success, fail, igvn); + Node* dom_cmp = ctrl->in(0)->in(1)->in(1); + Node* dom_val = dom_cmp->in(1); + if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val && ctrl->outcnt() == 1) { + ProjNode* success = NULL; + ProjNode* fail = NULL; + if (has_shared_region(proj, success, fail) && + // Next call modifies graph so must be last + fold_to_unsigned(proj, success, fail, igvn)) { + return this; + } + if (has_only_uncommon_traps(proj, success, fail, igvn) && + // Next call modifies graph so must be last + fold_to_unsigned(proj, success, fail, igvn)) { + return merge_uncommon_traps(proj, success, fail, igvn); + } } - return NULL; - } else if (ctrl->in(0) != NULL && - ctrl->in(0)->in(0) != NULL) { + } + if (ctrl->in(0) != NULL && + ctrl->in(0)->in(0) != NULL) { ProjNode* success = NULL; ProjNode* fail = NULL; Node* dom = ctrl->in(0)->in(0); - ProjNode* dom_cmp = dom->isa_Proj(); - ProjNode* other_cmp = ctrl->isa_Proj(); + ProjNode* dom_proj = dom->isa_Proj(); + ProjNode* other_proj = ctrl->isa_Proj(); // Check if it's an integer comparison dominated by another // integer comparison with another test in between - if (is_ctrl_folds(dom, igvn) && - has_only_uncommon_traps(dom_cmp, success, fail, igvn) && - is_side_effect_free_test(other_cmp, igvn) && - // Next call modifies graph so must be last - fold_compares_helper(dom_cmp, success, fail, igvn)) { - reroute_side_effect_free_unc(other_cmp, dom_cmp, igvn); - return merge_uncommon_traps(dom_cmp, success, fail, igvn); + if (is_ctrl_folds(dom, igvn)) { + if (fold_dominated_if(dom_proj, igvn)) { + return this; + } + Node* dom_cmp = dom->in(0)->in(1)->in(1); + Node* dom_val = dom_cmp->in(1); + if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val && + has_only_uncommon_traps(dom_proj, success, fail, igvn) && + is_side_effect_free_test(other_proj, igvn) && + // Next call modifies graph so must be last + fold_to_unsigned(dom_proj, success, fail, igvn)) { + reroute_side_effect_free_unc(other_proj, dom_proj, igvn); + return merge_uncommon_traps(dom_proj, success, fail, igvn); + } } } } diff --git a/test/hotspot/jtreg/compiler/c2/TestJumpTable.java b/test/hotspot/jtreg/compiler/c2/TestJumpTable.java index eac1bb53a897f..3c97f0735a022 100644 --- a/test/hotspot/jtreg/compiler/c2/TestJumpTable.java +++ b/test/hotspot/jtreg/compiler/c2/TestJumpTable.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,18 +23,24 @@ /** * @test - * @bug 8229855 + * @bug 8229855 8238812 * @summary Test jump table with key value that gets out of bounds after loop unrolling. * @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test* * -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -XX:-UseSwitchProfiling * compiler.c2.TestJumpTable + * @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test* + * -Xbatch -XX:-TieredCompilation -XX:-UseOnStackReplacement + * compiler.c2.TestJumpTable + * @run main/othervm -XX:CompileCommand=dontinline,compiler.c2.TestJumpTable::test* + * -Xbatch -XX:+UnlockDiagnosticVMOptions -XX:-TieredCompilation -XX:+StressIGVN + * compiler.c2.TestJumpTable */ package compiler.c2; public class TestJumpTable { - public static int test() { + public static int test0() { int res = 0; for (int i = 10; i < 50; ++i) { switch (i * 5) { @@ -53,9 +59,195 @@ public static int test() { return res; } + static int field; + + // Original (slightly simplified) fuzzer generated test + public static void test1() { + int i4, i5=99, i6, i9=89; + for (i4 = 12; i4 < 365; i4++) { + for (i6 = 5; i6 > 1; i6--) { + switch ((i6 * 5) + 11) { + case 13: + case 19: + case 26: + case 31: + case 35: + case 41: + case 43: + case 61: + case 71: + case 83: + case 314: + i9 = i5; + break; + } + } + } + } + + // This generates the following subgraph: + /* + // i: -10..4 + if ((i+min_jint) u<= max_jint) { <- This is always true but not folded by C2 + ... + } else { + ... + CastII(i-5, 0..45) <- Replaced by TOP because i-5 range is -15..-1 but still considered reachable by C2 although it is dead code + ... + } + */ + public static void test2() { + for (int i = 5; i > -10; i--) { + switch (i) { + case 0: + case 4: + case 10: + case 20: + case 30: + case 40: + case 50: + case 100: + field = 42; + break; + } + } + } + + // This generates the following subgraph: + /* + // i: -20..0 + if (i != 0) { + // i: -20..-1 + if (i < 0) { <- This is always true but not folded by C2 + // Fall through + } else { + ... + CastII(i-1, 0..4) <- Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code + ... + } + } else { + StoreI <- Due to this additional store on, IfNode::has_shared_region returns false and the fold compares optimization does not kick in + } + */ + public static void test3() { + for (int i = 5; i > -20; i -= 5) { + switch (i) { + case 0: + case 10: + case 20: + case 30: + case 40: + case 50: + case 60: + case 100: + field = 42; + break; + } + } + } + + // This generates the following subgraph: + /* + // i: -20..0 + if (i != 0) { + // i: -20..-1 + if (i u< 101) { <- This is always false but not folded by C2 because CmpU is not handled + CastII(i-1, 0..49) <- Replaced by TOP because i-1 range is -21..-1 but still considered reachable by C2 although it is dead code + } else { + ... + } + } else { + ... + } + */ + public static void test4() { + int local = 0; + for (int i = 5; i > -20; i -= 5) { + switch (i) { + case 0: + case 10: + case 20: + case 30: + case 40: + case 50: + case 100: + local = 42; + break; + } + } + } + + // This generates the following subgraph: + /* + // i: 0..20 + if (i != 20) { + // i: 0..19 + if ((i-20) u< 281) { <- This is always false but not folded by C2 because the two ifs compare different values + CastII(i-21, 0..49) <- Replaced by TOP because i-21 range is -21..-1 but still considered reachable by C2 although it is dead code + } else { + ... + } + } else { + ... + } + */ + public static void test5() { + int local; + for (int i = 25; i > 0; i -= 5) { + switch (i) { + case 20: + case 30: + case 40: + case 50: + case 60: + case 70: + case 300: + local = 42; + break; + } + } + } + + // This generates the following subgraph: + /* + // i: 0..20 + if ((i+10) != 30) { + // i: 0..19 + if ((i-20) u< 271) { <- This is always false but not folded by C2 because the two ifs compare different values + CastII(i-21, 0..4) <- Replaced by TOP because i-21 range is -21..-1 but still considered reachable by C2 although it is dead code + } else { + ... + } + } else { + ... + } + */ + public static void test6() { + int local; + for (int i = 25; i > 0; i -= 5) { + switch (i + 10) { + case 30: + case 40: + case 50: + case 60: + case 70: + case 80: + case 300: + local = 42; + break; + } + } + } + public static void main(String[] args) { - for (int i = 0; i < 20_000; ++i) { - test(); + for (int i = 0; i < 50_000; ++i) { + test0(); + test1(); + test2(); + test3(); + test4(); + test5(); + test6(); } } } From eff906a3f618bc61d9b22e54e8bbe4bea6481905 Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Sat, 27 Feb 2021 02:08:11 +0530 Subject: [PATCH 2/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/ifnode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 8a7381d403770..8f4e0f27ef143 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -859,7 +859,7 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod // There might be an AddINode (marked with *) with a constant increment // in-between the CmpNode(s) and the common value we compare. -// Check for the following cases. Then return common value compared +// Check for the following cases. Then return common value compared // if present and also save the constant values to be adjusted or // subtracted due to the possible AddINode in-between. // From 90ac1bda52e94521981705c07a63508850c6bc3e Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Tue, 2 Mar 2021 02:11:48 +0530 Subject: [PATCH 3/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/cfgnode.hpp | 2 +- src/hotspot/share/opto/ifnode.cpp | 122 ++++++++++++++--------------- 2 files changed, 58 insertions(+), 66 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 272c946ea5727..f3210e47e2f93 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -299,7 +299,7 @@ class IfNode : public MultiBranchNode { bool is_side_effect_free_test(ProjNode* proj, PhaseIterGVN* igvn); void reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, PhaseIterGVN* igvn); ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call) const; - Node* get_base_comparing_value(Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val); + bool get_base_comparing_value(Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val); bool fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn); bool fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); static bool is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* unc); diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 8f4e0f27ef143..7043005bf4780 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -669,7 +669,7 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj hi = TypeInt::INT->_hi; break; default: - assert(false, "should not reach here"); + ShouldNotReachHere(); break; } return TypeInt::make(lo, hi, cmp2_t->_widen); @@ -858,92 +858,84 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod } // There might be an AddINode (marked with *) with a constant increment -// in-between the CmpNode(s) and the common value we compare. -// Check for the following cases. Then return common value compared -// if present and also save the constant values to be adjusted or -// subtracted due to the possible AddINode in-between. +// in-between the CmpNodes and the common value we compare. +// Check for the following cases and return true if a common value is +// compared. Also save the constant value that is added to infer +// the type of the common value we compare. // // Variant 1 Variant 2 Variant 3 Variant 4 // -// res_val res_val res_val res_val -// / \ / \ / \ / \ -// dom_cmp \ dom_val* \ / this_val* dom_val* this_val* -// this_cmp | \ / \ | \ -// dom_cmp \ dom_cmp \ dom_cmp \ -// this_cmp this_cmp this_cmp -Node* IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) { - Node* this_cmp = in(1)->in(1); - Node* dom_cmp = dom_if->in(1)->in(1); - assert(this_cmp->is_Cmp() != false && dom_cmp->is_Cmp() != false, "compare expected"); - - Node* res_val = NULL; - this_adj_val = 0; - dom_adj_val = 0; +// res_val res_val res_val res_val +// / \ / \ / \ / \ +// dom_cmp \ / this_val* dom_val* \ dom_val* this_val* +// this_cmp / \ / \ | \ +// dom_cmp \ dom_cmp \ dom_cmp \ +// this_cmp this_cmp this_cmp +bool IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) { + Node* this_cmp = in(1)->in(1)->as_Cmp(); + Node* dom_cmp = dom_if->in(1)->in(1)->as_Cmp(); Node* dom_val = dom_cmp->in(1); Node* this_val = this_cmp->in(1); if (this_val == dom_val) { - res_val = this_val; - } else if (this_val->is_Add() && this_val->in(1) && this_val->in(1) == dom_val) { + // Variant 1 + return true; + } else if (this_val->is_Add() && this_val->in(1) != NULL && this_val->in(1) == dom_val) { const TypeInt* val_t = igvn->type(this_val->in(2))->isa_int(); - if (val_t && val_t->is_con()) { - this_adj_val = val_t->_lo; - res_val = this_val->in(1); + if (val_t != NULL && val_t->is_con()) { + // Variant 2 + this_adj_val = val_t->get_con(); + return true; } - } else if (dom_val->is_Add() && dom_val->in(1) && this_val == dom_val->in(1)) { + } else if (dom_val->is_Add() && dom_val->in(1) != NULL && this_val == dom_val->in(1)) { const TypeInt* val_t = igvn->type(dom_val->in(2))->isa_int(); - if (val_t && val_t->is_con()) { - dom_adj_val = val_t->_lo; - res_val = this_val; + if (val_t != NULL && val_t->is_con()) { + // Variant 3 + dom_adj_val = val_t->get_con(); + return true; } - } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) && dom_val->in(1) && this_val->in(1) == dom_val->in(1)) { + } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) != NULL && dom_val->in(1) != NULL && this_val->in(1) == dom_val->in(1)) { const TypeInt* domval_t = igvn->type(dom_val->in(2))->isa_int(); const TypeInt* thisval_t = igvn->type(this_val->in(2))->isa_int(); - if (thisval_t && domval_t && thisval_t->is_con() && domval_t->is_con()) { - this_adj_val = thisval_t->_lo; - dom_adj_val = domval_t->_lo; - res_val = this_val->in(1); + if (thisval_t != NULL && domval_t != NULL && thisval_t->is_con() && domval_t->is_con()) { + // Variant 4 + this_adj_val = thisval_t->get_con(); + dom_adj_val = domval_t->get_con(); + return true; } } - - return res_val; + return false; } // Check if dominating if determines the result of this if bool IfNode::fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn) { - Node* this_cmp = in(1)->in(1); + Node* this_val = in(1)->in(1)->in(1); Node* dom_if = proj->in(0)->as_If(); - Node* dom_cmp = dom_if->in(1)->in(1); + Node* dom_val = dom_if->in(1)->in(1)->in(1); jint this_adj_val = 0; jint dom_adj_val = 0; - Node* n = NULL; - n = get_base_comparing_value(dom_if, igvn, this_adj_val, dom_adj_val); - if (n == NULL) { - // Not comparing same value - return false; - } - - const TypeInt* failtype = filtered_int_type(igvn, dom_cmp->in(1), proj); - if (failtype != NULL) { - if (dom_adj_val != 0) { - // To account for the AddINode, subtract the constant increment from the type - assert(dom_cmp->in(1)->is_Add() != false, "sanity"); - failtype = dom_cmp->in(1)->as_Add()->add_ring(failtype, TypeInt::make(-dom_adj_val))->is_int(); - } - for (int i = 0; i < 2; ++i) { - const TypeInt* type2 = filtered_int_type(igvn, this_cmp->in(1), proj_out(i)); - if (type2 != NULL) { - if (this_adj_val != 0) { - // To account for the AddINode, subtract the constant increment from the type - assert(this_cmp->in(1)->is_Add() != false, "sanity"); - type2 = this_cmp->in(1)->as_Add()->add_ring(type2, TypeInt::make(-this_adj_val))->is_int(); - } - const TypeInt* res_type = failtype->join(type2)->is_int(); - if (res_type->empty()) { - // Replace Bool with constant - igvn->_worklist.push(in(1)); - igvn->replace_input_of(this, 1, igvn->intcon(proj_out(1-i)->_con)); - return true; + // Must compare same value + if (get_base_comparing_value(dom_if, igvn, this_adj_val, dom_adj_val)) { + const TypeInt* failtype = filtered_int_type(igvn, dom_val, proj); + if (failtype != NULL) { + if (dom_adj_val != 0) { + // To account for the AddINode, subtract the constant increment from the type + failtype = dom_val->as_Add()->add_ring(failtype, TypeInt::make(-dom_adj_val))->is_int(); + } + for (int i = 0; i < 2; ++i) { + const TypeInt* type = filtered_int_type(igvn, this_val, proj_out(i)); + if (type != NULL) { + if (this_adj_val != 0) { + // To account for the AddINode, subtract the constant increment from the type + type = this_val->as_Add()->add_ring(type, TypeInt::make(-this_adj_val))->is_int(); + } + type = failtype->join(type)->is_int(); + if (type->empty()) { + // Replace Bool with constant + igvn->_worklist.push(in(1)); + igvn->replace_input_of(this, 1, igvn->intcon(proj_out(1-i)->_con)); + return true; + } } } } From 5cef73ad4b26a398f0ffab28a01a7f7799d19a6b Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Wed, 3 Mar 2021 16:52:39 +0530 Subject: [PATCH 4/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/cfgnode.hpp | 2 +- src/hotspot/share/opto/ifnode.cpp | 28 ++++++++++--------- .../jtreg/compiler/c2/TestJumpTable.java | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index f3210e47e2f93..43f3897eb45e0 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -288,7 +288,7 @@ class IfNode : public MultiBranchNode { private: // Helper methods for fold_compares - bool cmpi_folds(PhaseIterGVN* igvn); + bool cmp_folds(PhaseIterGVN* igvn); bool is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn); bool has_shared_region(ProjNode* proj, ProjNode*& success, ProjNode*& fail); bool has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNode*& fail, PhaseIterGVN* igvn); diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 7043005bf4780..6f5844f509a09 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -634,9 +634,10 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj return cmp2_t; case BoolTest::lt: if (is_unsigned && lo >= 0) { - // val u< cmp2 only passes for val >= 0 if cmp2 >= 0 + // cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0. lo = 0; } else { + // The lower bound of val cannot be improved. lo = TypeInt::INT->_lo; } if (hi != min_jint) { @@ -645,9 +646,10 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj break; case BoolTest::le: if (is_unsigned && lo >= 0) { - // val u<= cmp2 only passes for val >= 0 if cmp2 >= 0 + // cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0. lo = 0; } else { + // The lower bound of val cannot be improved. lo = TypeInt::INT->_lo; } break; @@ -661,11 +663,11 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj hi = TypeInt::INT->_hi; break; case BoolTest::ge: - // lo unchanged if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) { // val u>= cmp2 passes for val < 0 lo = TypeInt::INT->_lo; } + // else lo unchanged hi = TypeInt::INT->_hi; break; default: @@ -720,7 +722,7 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj // // Is the comparison for this If suitable for folding? -bool IfNode::cmpi_folds(PhaseIterGVN* igvn) { +bool IfNode::cmp_folds(PhaseIterGVN* igvn) { return in(1) != NULL && in(1)->is_Bool() && in(1)->in(1) != NULL && @@ -740,7 +742,7 @@ bool IfNode::is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn) { ctrl->in(0) != NULL && ctrl->in(0)->Opcode() == Op_If && ctrl->in(0)->outcnt() == 2 && - ctrl->in(0)->as_If()->cmpi_folds(igvn) && + ctrl->in(0)->as_If()->cmp_folds(igvn) && ctrl->in(0)->in(1)->in(1)->in(1) != NULL && in(1)->in(1)->in(1) != NULL; } @@ -872,28 +874,28 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod // dom_cmp \ dom_cmp \ dom_cmp \ // this_cmp this_cmp this_cmp bool IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) { - Node* this_cmp = in(1)->in(1)->as_Cmp(); - Node* dom_cmp = dom_if->in(1)->in(1)->as_Cmp(); - Node* dom_val = dom_cmp->in(1); - Node* this_val = this_cmp->in(1); + assert(dom_if->in(1)->in(1)->is_Cmp() && in(1)->in(1)->is_Cmp(), "compare expected"); + Node* dom_val = dom_if->in(1)->in(1)->in(1); + Node* this_val = in(1)->in(1)->in(1); + assert(dom_val != NULL && this_val != NULL, "sanity"); if (this_val == dom_val) { // Variant 1 return true; - } else if (this_val->is_Add() && this_val->in(1) != NULL && this_val->in(1) == dom_val) { + } else if (this_val->is_Add() && this_val->in(1) == dom_val) { const TypeInt* val_t = igvn->type(this_val->in(2))->isa_int(); if (val_t != NULL && val_t->is_con()) { // Variant 2 this_adj_val = val_t->get_con(); return true; } - } else if (dom_val->is_Add() && dom_val->in(1) != NULL && this_val == dom_val->in(1)) { + } else if (dom_val->is_Add() && this_val == dom_val->in(1)) { const TypeInt* val_t = igvn->type(dom_val->in(2))->isa_int(); if (val_t != NULL && val_t->is_con()) { // Variant 3 dom_adj_val = val_t->get_con(); return true; } - } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) != NULL && dom_val->in(1) != NULL && this_val->in(1) == dom_val->in(1)) { + } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) != NULL && this_val->in(1) == dom_val->in(1)) { const TypeInt* domval_t = igvn->type(dom_val->in(2))->isa_int(); const TypeInt* thisval_t = igvn->type(this_val->in(2))->isa_int(); if (thisval_t != NULL && domval_t != NULL && thisval_t->is_con() && domval_t->is_con()) { @@ -1357,7 +1359,7 @@ void IfNode::reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, Ph Node* IfNode::fold_compares(PhaseIterGVN* igvn) { if (Opcode() != Op_If) return NULL; - if (cmpi_folds(igvn)) { + if (cmp_folds(igvn)) { Node* ctrl = in(0); Node* cmp = in(1)->in(1); Node* val = cmp->in(1); diff --git a/test/hotspot/jtreg/compiler/c2/TestJumpTable.java b/test/hotspot/jtreg/compiler/c2/TestJumpTable.java index 3c97f0735a022..a7d1a8034521f 100644 --- a/test/hotspot/jtreg/compiler/c2/TestJumpTable.java +++ b/test/hotspot/jtreg/compiler/c2/TestJumpTable.java @@ -63,7 +63,7 @@ public static int test0() { // Original (slightly simplified) fuzzer generated test public static void test1() { - int i4, i5=99, i6, i9=89; + int i4, i5 = 99, i6, i9 = 89; for (i4 = 12; i4 < 365; i4++) { for (i6 = 5; i6 > 1; i6--) { switch ((i6 * 5) + 11) { From b80e1b6c33a0ae43ea146e40baa3995cf622c81d Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Tue, 9 Mar 2021 14:57:14 +0530 Subject: [PATCH 5/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/cfgnode.hpp | 8 +- src/hotspot/share/opto/ifnode.cpp | 233 ++++++++--------------------- src/hotspot/share/opto/parse2.cpp | 9 +- 3 files changed, 70 insertions(+), 180 deletions(-) diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 43f3897eb45e0..d0a22b78b6e54 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -288,7 +288,7 @@ class IfNode : public MultiBranchNode { private: // Helper methods for fold_compares - bool cmp_folds(PhaseIterGVN* igvn); + bool cmpi_folds(PhaseIterGVN* igvn, bool fold_ne = false); bool is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn); bool has_shared_region(ProjNode* proj, ProjNode*& success, ProjNode*& fail); bool has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNode*& fail, PhaseIterGVN* igvn); @@ -299,9 +299,7 @@ class IfNode : public MultiBranchNode { bool is_side_effect_free_test(ProjNode* proj, PhaseIterGVN* igvn); void reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, PhaseIterGVN* igvn); ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call) const; - bool get_base_comparing_value(Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val); - bool fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn); - bool fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); + bool fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); static bool is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* unc); protected: diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 6f5844f509a09..482ccd63740e1 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -606,19 +606,17 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj 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(); + 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 != NULL) { jint lo = cmp2_t->_lo; jint hi = cmp2_t->_hi; - const TypeInt* val_t = gvn->type(val)->isa_int(); - bool is_unsigned = (cmp->Opcode() == Op_CmpU); BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate(); switch (msk) { case BoolTest::ne: { - assert(!is_unsigned, "unsigned comparison is not supported"); // 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 != NULL && !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); @@ -630,51 +628,31 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj return NULL; } case BoolTest::eq: - assert(!is_unsigned, "unsigned comparison is not supported"); return cmp2_t; case BoolTest::lt: - if (is_unsigned && lo >= 0) { - // cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0. - lo = 0; - } else { - // The lower bound of val cannot be improved. - lo = TypeInt::INT->_lo; - } + lo = TypeInt::INT->_lo; if (hi != min_jint) { hi = hi - 1; } break; case BoolTest::le: - if (is_unsigned && lo >= 0) { - // cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0. - lo = 0; - } else { - // The lower bound of val cannot be improved. - lo = TypeInt::INT->_lo; - } + lo = TypeInt::INT->_lo; break; case BoolTest::gt: - if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) { - // val u> cmp2 passes for val < 0 - lo = TypeInt::INT->_lo; - } else if (lo != max_jint) { + if (lo != max_jint) { lo = lo + 1; } hi = TypeInt::INT->_hi; break; case BoolTest::ge: - if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) { - // val u>= cmp2 passes for val < 0 - lo = TypeInt::INT->_lo; - } - // else lo unchanged + // lo unchanged hi = TypeInt::INT->_hi; break; default: - ShouldNotReachHere(); break; } - return TypeInt::make(lo, hi, cmp2_t->_widen); + const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen); + return rtn_t; } } } @@ -722,17 +700,16 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj // // Is the comparison for this If suitable for folding? -bool IfNode::cmp_folds(PhaseIterGVN* igvn) { +bool IfNode::cmpi_folds(PhaseIterGVN* igvn, bool fold_ne) { return in(1) != NULL && in(1)->is_Bool() && in(1)->in(1) != NULL && - (in(1)->in(1)->Opcode() == Op_CmpI || - in(1)->in(1)->Opcode() == Op_CmpU) && + in(1)->in(1)->Opcode() == Op_CmpI && in(1)->in(1)->in(2) != NULL && in(1)->in(1)->in(2) != igvn->C->top() && (in(1)->as_Bool()->_test.is_less() || in(1)->as_Bool()->_test.is_greater() || - in(1)->as_Bool()->_test._test == BoolTest::ne); + (fold_ne && in(1)->as_Bool()->_test._test == BoolTest::ne)); } // Is a dominating control suitable for folding with this if? @@ -742,9 +719,10 @@ bool IfNode::is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn) { ctrl->in(0) != NULL && ctrl->in(0)->Opcode() == Op_If && ctrl->in(0)->outcnt() == 2 && - ctrl->in(0)->as_If()->cmp_folds(igvn) && + ctrl->in(0)->as_If()->cmpi_folds(igvn, true) && + // Must compare same value ctrl->in(0)->in(1)->in(1)->in(1) != NULL && - in(1)->in(1)->in(1) != NULL; + ctrl->in(0)->in(1)->in(1)->in(1) == in(1)->in(1)->in(1); } // Do this If and the dominating If share a region? @@ -859,94 +837,8 @@ bool IfNode::has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNod return false; } -// There might be an AddINode (marked with *) with a constant increment -// in-between the CmpNodes and the common value we compare. -// Check for the following cases and return true if a common value is -// compared. Also save the constant value that is added to infer -// the type of the common value we compare. -// -// Variant 1 Variant 2 Variant 3 Variant 4 -// -// res_val res_val res_val res_val -// / \ / \ / \ / \ -// dom_cmp \ / this_val* dom_val* \ dom_val* this_val* -// this_cmp / \ / \ | \ -// dom_cmp \ dom_cmp \ dom_cmp \ -// this_cmp this_cmp this_cmp -bool IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) { - assert(dom_if->in(1)->in(1)->is_Cmp() && in(1)->in(1)->is_Cmp(), "compare expected"); - Node* dom_val = dom_if->in(1)->in(1)->in(1); - Node* this_val = in(1)->in(1)->in(1); - assert(dom_val != NULL && this_val != NULL, "sanity"); - if (this_val == dom_val) { - // Variant 1 - return true; - } else if (this_val->is_Add() && this_val->in(1) == dom_val) { - const TypeInt* val_t = igvn->type(this_val->in(2))->isa_int(); - if (val_t != NULL && val_t->is_con()) { - // Variant 2 - this_adj_val = val_t->get_con(); - return true; - } - } else if (dom_val->is_Add() && this_val == dom_val->in(1)) { - const TypeInt* val_t = igvn->type(dom_val->in(2))->isa_int(); - if (val_t != NULL && val_t->is_con()) { - // Variant 3 - dom_adj_val = val_t->get_con(); - return true; - } - } else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) != NULL && this_val->in(1) == dom_val->in(1)) { - const TypeInt* domval_t = igvn->type(dom_val->in(2))->isa_int(); - const TypeInt* thisval_t = igvn->type(this_val->in(2))->isa_int(); - if (thisval_t != NULL && domval_t != NULL && thisval_t->is_con() && domval_t->is_con()) { - // Variant 4 - this_adj_val = thisval_t->get_con(); - dom_adj_val = domval_t->get_con(); - return true; - } - } - return false; -} - -// Check if dominating if determines the result of this if -bool IfNode::fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn) { - Node* this_val = in(1)->in(1)->in(1); - Node* dom_if = proj->in(0)->as_If(); - Node* dom_val = dom_if->in(1)->in(1)->in(1); - jint this_adj_val = 0; - jint dom_adj_val = 0; - - // Must compare same value - if (get_base_comparing_value(dom_if, igvn, this_adj_val, dom_adj_val)) { - const TypeInt* failtype = filtered_int_type(igvn, dom_val, proj); - if (failtype != NULL) { - if (dom_adj_val != 0) { - // To account for the AddINode, subtract the constant increment from the type - failtype = dom_val->as_Add()->add_ring(failtype, TypeInt::make(-dom_adj_val))->is_int(); - } - for (int i = 0; i < 2; ++i) { - const TypeInt* type = filtered_int_type(igvn, this_val, proj_out(i)); - if (type != NULL) { - if (this_adj_val != 0) { - // To account for the AddINode, subtract the constant increment from the type - type = this_val->as_Add()->add_ring(type, TypeInt::make(-this_adj_val))->is_int(); - } - type = failtype->join(type)->is_int(); - if (type->empty()) { - // Replace Bool with constant - igvn->_worklist.push(in(1)); - igvn->replace_input_of(this, 1, igvn->intcon(proj_out(1-i)->_con)); - return true; - } - } - } - } - } - return false; -} - // Check that the 2 CmpI can be folded into as single CmpU and proceed with the folding -bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) { +bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) { Node* this_cmp = in(1)->in(1); BoolNode* this_bool = in(1)->as_Bool(); IfNode* dom_iff = proj->in(0)->as_If(); @@ -955,17 +847,13 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, Node* hi = this_cmp->in(2); Node* n = this_cmp->in(1); ProjNode* otherproj = proj->other_if_proj(); - assert(this_cmp->Opcode() == Op_CmpI && dom_iff->in(1)->in(1)->Opcode() == Op_CmpI, "Unexpected CmpNode"); + + const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj); + const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success); BoolTest::mask lo_test = dom_bool->_test._test; BoolTest::mask hi_test = this_bool->_test._test; BoolTest::mask cond = hi_test; - if (lo_test == BoolTest::ne || hi_test == BoolTest::ne) { - return false; - } - - const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj); - const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success); // convert: // @@ -993,7 +881,7 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, // sets the lower bound if any. Node* adjusted_lim = NULL; if (lo_type != NULL && hi_type != NULL && hi_type->_lo > lo_type->_hi && - hi_type->_hi == max_jint && lo_type->_lo == min_jint) { + hi_type->_hi == max_jint && lo_type->_lo == min_jint && lo_test != BoolTest::ne) { assert((dom_bool->_test.is_less() && !proj->_con) || (dom_bool->_test.is_greater() && proj->_con), "incorrect test"); // this test was canonicalized @@ -1038,7 +926,7 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, return false; } } else if (lo_type != NULL && hi_type != NULL && lo_type->_lo > hi_type->_hi && - lo_type->_hi == max_jint && hi_type->_lo == min_jint) { + lo_type->_hi == max_jint && hi_type->_lo == min_jint && lo_test != BoolTest::ne) { // this_bool = < // dom_bool = < (proj = True) or dom_bool = >= (proj = False) @@ -1096,6 +984,20 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, return false; } } else { + const TypeInt* failtype = filtered_int_type(igvn, n, proj); + if (failtype != NULL) { + const TypeInt* type2 = filtered_int_type(igvn, n, fail); + if (type2 != NULL) { + failtype = failtype->join(type2)->is_int(); + if (failtype->_lo > failtype->_hi) { + // previous if determines the result of this if so + // replace Bool with constant + igvn->_worklist.push(in(1)); + igvn->replace_input_of(this, 1, igvn->intcon(success->_con)); + return true; + } + } + } lo = NULL; hi = NULL; } @@ -1359,57 +1261,42 @@ void IfNode::reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, Ph Node* IfNode::fold_compares(PhaseIterGVN* igvn) { if (Opcode() != Op_If) return NULL; - if (cmp_folds(igvn)) { + if (cmpi_folds(igvn)) { Node* ctrl = in(0); - Node* cmp = in(1)->in(1); - Node* val = cmp->in(1); - // An integer comparison immediately dominated by another integer comparison - if (is_ctrl_folds(ctrl, igvn)) { - ProjNode* proj = ctrl->as_Proj(); - if (fold_dominated_if(proj, igvn)) { + if (is_ctrl_folds(ctrl, igvn) && ctrl->outcnt() == 1) { + // A integer comparison immediately dominated by another integer + // comparison + ProjNode* success = NULL; + ProjNode* fail = NULL; + ProjNode* dom_cmp = ctrl->as_Proj(); + if (has_shared_region(dom_cmp, success, fail) && + // Next call modifies graph so must be last + fold_compares_helper(dom_cmp, success, fail, igvn)) { return this; } - Node* dom_cmp = ctrl->in(0)->in(1)->in(1); - Node* dom_val = dom_cmp->in(1); - if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val && ctrl->outcnt() == 1) { - ProjNode* success = NULL; - ProjNode* fail = NULL; - if (has_shared_region(proj, success, fail) && - // Next call modifies graph so must be last - fold_to_unsigned(proj, success, fail, igvn)) { - return this; - } - if (has_only_uncommon_traps(proj, success, fail, igvn) && - // Next call modifies graph so must be last - fold_to_unsigned(proj, success, fail, igvn)) { - return merge_uncommon_traps(proj, success, fail, igvn); - } + if (has_only_uncommon_traps(dom_cmp, success, fail, igvn) && + // Next call modifies graph so must be last + fold_compares_helper(dom_cmp, success, fail, igvn)) { + return merge_uncommon_traps(dom_cmp, success, fail, igvn); } - } - if (ctrl->in(0) != NULL && - ctrl->in(0)->in(0) != NULL) { + return NULL; + } else if (ctrl->in(0) != NULL && + ctrl->in(0)->in(0) != NULL) { ProjNode* success = NULL; ProjNode* fail = NULL; Node* dom = ctrl->in(0)->in(0); - ProjNode* dom_proj = dom->isa_Proj(); - ProjNode* other_proj = ctrl->isa_Proj(); + ProjNode* dom_cmp = dom->isa_Proj(); + ProjNode* other_cmp = ctrl->isa_Proj(); // Check if it's an integer comparison dominated by another // integer comparison with another test in between - if (is_ctrl_folds(dom, igvn)) { - if (fold_dominated_if(dom_proj, igvn)) { - return this; - } - Node* dom_cmp = dom->in(0)->in(1)->in(1); - Node* dom_val = dom_cmp->in(1); - if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val && - has_only_uncommon_traps(dom_proj, success, fail, igvn) && - is_side_effect_free_test(other_proj, igvn) && - // Next call modifies graph so must be last - fold_to_unsigned(dom_proj, success, fail, igvn)) { - reroute_side_effect_free_unc(other_proj, dom_proj, igvn); - return merge_uncommon_traps(dom_proj, success, fail, igvn); - } + if (is_ctrl_folds(dom, igvn) && + has_only_uncommon_traps(dom_cmp, success, fail, igvn) && + is_side_effect_free_test(other_cmp, igvn) && + // Next call modifies graph so must be last + fold_compares_helper(dom_cmp, success, fail, igvn)) { + reroute_side_effect_free_unc(other_cmp, dom_cmp, igvn); + return merge_uncommon_traps(dom_cmp, success, fail, igvn); } } } diff --git a/src/hotspot/share/opto/parse2.cpp b/src/hotspot/share/opto/parse2.cpp index 31f816cb68510..abe09fa89a290 100644 --- a/src/hotspot/share/opto/parse2.cpp +++ b/src/hotspot/share/opto/parse2.cpp @@ -863,10 +863,15 @@ bool Parse::create_jump_tables(Node* key_val, SwitchRange* lo, SwitchRange* hi) // Clean the 32-bit int into a real 64-bit offset. // Otherwise, the jint value 0 might turn into an offset of 0x0800000000. - const TypeInt* ikeytype = TypeInt::make(0, num_cases, Type::WidenMin); // Make I2L conversion control dependent to prevent it from // floating above the range check during loop optimizations. - key_val = C->conv_I2X_index(&_gvn, key_val, ikeytype, control()); + // Do not use a narrow int type here to prevent the data path from dying + // while the control path is not removed. This can happen if the type of key_val + // is later known to be out of bounds of [0, num_cases] and therefore a narrow cast + // would be replaced by TOP while C2 is not able to fold the corresponding range checks. +#ifdef _LP64 + key_val = C->constrained_convI2L(&_gvn, key_val, TypeInt::INT, control()); +#endif // Shift the value by wordsize so we have an index into the table, rather // than a switch value From b027ab4ac582b767a478d4ca2c7943e39e2c4838 Mon Sep 17 00:00:00 2001 From: Rahul Raghavan Date: Tue, 9 Mar 2021 20:03:20 +0530 Subject: [PATCH 6/6] 8238812: assert(false) failed: bad AD file --- src/hotspot/share/opto/compile.cpp | 4 ++-- src/hotspot/share/opto/compile.hpp | 2 +- src/hotspot/share/opto/parse2.cpp | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 7fa125351e29d..6c830b072631f 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -4161,10 +4161,10 @@ Node* Compile::conv_I2X_index(PhaseGVN* phase, Node* idx, const TypeInt* sizetyp } // Convert integer value to a narrowed long type dependent on ctrl (for example, a range check) -Node* Compile::constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl) { +Node* Compile::constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl, bool carry_dependency) { if (ctrl != NULL) { // Express control dependency by a CastII node with a narrow type. - value = new CastIINode(value, itype, false, true /* range check dependency */); + value = new CastIINode(value, itype, carry_dependency, true /* range check dependency */); // Make the CastII node dependent on the control input to prevent the narrowed ConvI2L // node from floating above the range check during loop optimizations. Otherwise, the // ConvI2L node may be eliminated independently of the range check, causing the data path diff --git a/src/hotspot/share/opto/compile.hpp b/src/hotspot/share/opto/compile.hpp index ae9d6a3488fe2..513410f234016 100644 --- a/src/hotspot/share/opto/compile.hpp +++ b/src/hotspot/share/opto/compile.hpp @@ -1157,7 +1157,7 @@ class Compile : public Phase { Node* ctrl = NULL); // Convert integer value to a narrowed long type dependent on ctrl (for example, a range check) - static Node* constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl); + static Node* constrained_convI2L(PhaseGVN* phase, Node* value, const TypeInt* itype, Node* ctrl, bool carry_dependency = false); // Auxiliary methods for randomized fuzzing/stressing int random(); diff --git a/src/hotspot/share/opto/parse2.cpp b/src/hotspot/share/opto/parse2.cpp index abe09fa89a290..0ca06c4b83435 100644 --- a/src/hotspot/share/opto/parse2.cpp +++ b/src/hotspot/share/opto/parse2.cpp @@ -869,8 +869,9 @@ bool Parse::create_jump_tables(Node* key_val, SwitchRange* lo, SwitchRange* hi) // while the control path is not removed. This can happen if the type of key_val // is later known to be out of bounds of [0, num_cases] and therefore a narrow cast // would be replaced by TOP while C2 is not able to fold the corresponding range checks. + // Set _carry_dependency for the cast to avoid being removed by IGVN. #ifdef _LP64 - key_val = C->constrained_convI2L(&_gvn, key_val, TypeInt::INT, control()); + key_val = C->constrained_convI2L(&_gvn, key_val, TypeInt::INT, control(), true /* carry_dependency */); #endif // Shift the value by wordsize so we have an index into the table, rather