Skip to content

Commit b80e1b6

Browse files
author
Rahul Raghavan
committed
8238812: assert(false) failed: bad AD file
1 parent 5cef73a commit b80e1b6

File tree

3 files changed

+70
-180
lines changed

3 files changed

+70
-180
lines changed

src/hotspot/share/opto/cfgnode.hpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -288,7 +288,7 @@ class IfNode : public MultiBranchNode {
288288

289289
private:
290290
// Helper methods for fold_compares
291-
bool cmp_folds(PhaseIterGVN* igvn);
291+
bool cmpi_folds(PhaseIterGVN* igvn, bool fold_ne = false);
292292
bool is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn);
293293
bool has_shared_region(ProjNode* proj, ProjNode*& success, ProjNode*& fail);
294294
bool has_only_uncommon_traps(ProjNode* proj, ProjNode*& success, ProjNode*& fail, PhaseIterGVN* igvn);
@@ -299,9 +299,7 @@ class IfNode : public MultiBranchNode {
299299
bool is_side_effect_free_test(ProjNode* proj, PhaseIterGVN* igvn);
300300
void reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, PhaseIterGVN* igvn);
301301
ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call) const;
302-
bool get_base_comparing_value(Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val);
303-
bool fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn);
304-
bool fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn);
302+
bool fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn);
305303
static bool is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* unc);
306304

307305
protected:

src/hotspot/share/opto/ifnode.cpp

Lines changed: 60 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -606,19 +606,17 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj
606606
if (iff->in(1) && iff->in(1)->is_Bool()) {
607607
BoolNode* bol = iff->in(1)->as_Bool();
608608
if (bol->in(1) && bol->in(1)->is_Cmp()) {
609-
const CmpNode* cmp = bol->in(1)->as_Cmp();
609+
const CmpNode* cmp = bol->in(1)->as_Cmp();
610610
if (cmp->in(1) == val) {
611611
const TypeInt* cmp2_t = gvn->type(cmp->in(2))->isa_int();
612612
if (cmp2_t != NULL) {
613613
jint lo = cmp2_t->_lo;
614614
jint hi = cmp2_t->_hi;
615-
const TypeInt* val_t = gvn->type(val)->isa_int();
616-
bool is_unsigned = (cmp->Opcode() == Op_CmpU);
617615
BoolTest::mask msk = if_proj->Opcode() == Op_IfTrue ? bol->_test._test : bol->_test.negate();
618616
switch (msk) {
619617
case BoolTest::ne: {
620-
assert(!is_unsigned, "unsigned comparison is not supported");
621618
// If val is compared to its lower or upper bound, we can narrow the type
619+
const TypeInt* val_t = gvn->type(val)->isa_int();
622620
if (val_t != NULL && !val_t->singleton() && cmp2_t->is_con()) {
623621
if (val_t->_lo == lo) {
624622
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
630628
return NULL;
631629
}
632630
case BoolTest::eq:
633-
assert(!is_unsigned, "unsigned comparison is not supported");
634631
return cmp2_t;
635632
case BoolTest::lt:
636-
if (is_unsigned && lo >= 0) {
637-
// cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0.
638-
lo = 0;
639-
} else {
640-
// The lower bound of val cannot be improved.
641-
lo = TypeInt::INT->_lo;
642-
}
633+
lo = TypeInt::INT->_lo;
643634
if (hi != min_jint) {
644635
hi = hi - 1;
645636
}
646637
break;
647638
case BoolTest::le:
648-
if (is_unsigned && lo >= 0) {
649-
// cmp2 >= 0: val u<= cmp2 can only pass if val >= 0. Set val->_lo to 0.
650-
lo = 0;
651-
} else {
652-
// The lower bound of val cannot be improved.
653-
lo = TypeInt::INT->_lo;
654-
}
639+
lo = TypeInt::INT->_lo;
655640
break;
656641
case BoolTest::gt:
657-
if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) {
658-
// val u> cmp2 passes for val < 0
659-
lo = TypeInt::INT->_lo;
660-
} else if (lo != max_jint) {
642+
if (lo != max_jint) {
661643
lo = lo + 1;
662644
}
663645
hi = TypeInt::INT->_hi;
664646
break;
665647
case BoolTest::ge:
666-
if (is_unsigned && (val_t == NULL || val_t->_lo < 0)) {
667-
// val u>= cmp2 passes for val < 0
668-
lo = TypeInt::INT->_lo;
669-
}
670-
// else lo unchanged
648+
// lo unchanged
671649
hi = TypeInt::INT->_hi;
672650
break;
673651
default:
674-
ShouldNotReachHere();
675652
break;
676653
}
677-
return TypeInt::make(lo, hi, cmp2_t->_widen);
654+
const TypeInt* rtn_t = TypeInt::make(lo, hi, cmp2_t->_widen);
655+
return rtn_t;
678656
}
679657
}
680658
}
@@ -722,17 +700,16 @@ const TypeInt* IfNode::filtered_int_type(PhaseGVN* gvn, Node* val, Node* if_proj
722700
//
723701

724702
// Is the comparison for this If suitable for folding?
725-
bool IfNode::cmp_folds(PhaseIterGVN* igvn) {
703+
bool IfNode::cmpi_folds(PhaseIterGVN* igvn, bool fold_ne) {
726704
return in(1) != NULL &&
727705
in(1)->is_Bool() &&
728706
in(1)->in(1) != NULL &&
729-
(in(1)->in(1)->Opcode() == Op_CmpI ||
730-
in(1)->in(1)->Opcode() == Op_CmpU) &&
707+
in(1)->in(1)->Opcode() == Op_CmpI &&
731708
in(1)->in(1)->in(2) != NULL &&
732709
in(1)->in(1)->in(2) != igvn->C->top() &&
733710
(in(1)->as_Bool()->_test.is_less() ||
734711
in(1)->as_Bool()->_test.is_greater() ||
735-
in(1)->as_Bool()->_test._test == BoolTest::ne);
712+
(fold_ne && in(1)->as_Bool()->_test._test == BoolTest::ne));
736713
}
737714

738715
// Is a dominating control suitable for folding with this if?
@@ -742,9 +719,10 @@ bool IfNode::is_ctrl_folds(Node* ctrl, PhaseIterGVN* igvn) {
742719
ctrl->in(0) != NULL &&
743720
ctrl->in(0)->Opcode() == Op_If &&
744721
ctrl->in(0)->outcnt() == 2 &&
745-
ctrl->in(0)->as_If()->cmp_folds(igvn) &&
722+
ctrl->in(0)->as_If()->cmpi_folds(igvn, true) &&
723+
// Must compare same value
746724
ctrl->in(0)->in(1)->in(1)->in(1) != NULL &&
747-
in(1)->in(1)->in(1) != NULL;
725+
ctrl->in(0)->in(1)->in(1)->in(1) == in(1)->in(1)->in(1);
748726
}
749727

750728
// 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
859837
return false;
860838
}
861839

862-
// There might be an AddINode (marked with *) with a constant increment
863-
// in-between the CmpNodes and the common value we compare.
864-
// Check for the following cases and return true if a common value is
865-
// compared. Also save the constant value that is added to infer
866-
// the type of the common value we compare.
867-
//
868-
// Variant 1 Variant 2 Variant 3 Variant 4
869-
//
870-
// res_val res_val res_val res_val
871-
// / \ / \ / \ / \
872-
// dom_cmp \ / this_val* dom_val* \ dom_val* this_val*
873-
// this_cmp / \ / \ | \
874-
// dom_cmp \ dom_cmp \ dom_cmp \
875-
// this_cmp this_cmp this_cmp
876-
bool IfNode::get_base_comparing_value (Node* dom_if, PhaseIterGVN* igvn, jint& this_adj_val, jint& dom_adj_val) {
877-
assert(dom_if->in(1)->in(1)->is_Cmp() && in(1)->in(1)->is_Cmp(), "compare expected");
878-
Node* dom_val = dom_if->in(1)->in(1)->in(1);
879-
Node* this_val = in(1)->in(1)->in(1);
880-
assert(dom_val != NULL && this_val != NULL, "sanity");
881-
if (this_val == dom_val) {
882-
// Variant 1
883-
return true;
884-
} else if (this_val->is_Add() && this_val->in(1) == dom_val) {
885-
const TypeInt* val_t = igvn->type(this_val->in(2))->isa_int();
886-
if (val_t != NULL && val_t->is_con()) {
887-
// Variant 2
888-
this_adj_val = val_t->get_con();
889-
return true;
890-
}
891-
} else if (dom_val->is_Add() && this_val == dom_val->in(1)) {
892-
const TypeInt* val_t = igvn->type(dom_val->in(2))->isa_int();
893-
if (val_t != NULL && val_t->is_con()) {
894-
// Variant 3
895-
dom_adj_val = val_t->get_con();
896-
return true;
897-
}
898-
} else if (this_val->is_Add() && dom_val->is_Add() && this_val->in(1) != NULL && this_val->in(1) == dom_val->in(1)) {
899-
const TypeInt* domval_t = igvn->type(dom_val->in(2))->isa_int();
900-
const TypeInt* thisval_t = igvn->type(this_val->in(2))->isa_int();
901-
if (thisval_t != NULL && domval_t != NULL && thisval_t->is_con() && domval_t->is_con()) {
902-
// Variant 4
903-
this_adj_val = thisval_t->get_con();
904-
dom_adj_val = domval_t->get_con();
905-
return true;
906-
}
907-
}
908-
return false;
909-
}
910-
911-
// Check if dominating if determines the result of this if
912-
bool IfNode::fold_dominated_if(ProjNode* proj, PhaseIterGVN* igvn) {
913-
Node* this_val = in(1)->in(1)->in(1);
914-
Node* dom_if = proj->in(0)->as_If();
915-
Node* dom_val = dom_if->in(1)->in(1)->in(1);
916-
jint this_adj_val = 0;
917-
jint dom_adj_val = 0;
918-
919-
// Must compare same value
920-
if (get_base_comparing_value(dom_if, igvn, this_adj_val, dom_adj_val)) {
921-
const TypeInt* failtype = filtered_int_type(igvn, dom_val, proj);
922-
if (failtype != NULL) {
923-
if (dom_adj_val != 0) {
924-
// To account for the AddINode, subtract the constant increment from the type
925-
failtype = dom_val->as_Add()->add_ring(failtype, TypeInt::make(-dom_adj_val))->is_int();
926-
}
927-
for (int i = 0; i < 2; ++i) {
928-
const TypeInt* type = filtered_int_type(igvn, this_val, proj_out(i));
929-
if (type != NULL) {
930-
if (this_adj_val != 0) {
931-
// To account for the AddINode, subtract the constant increment from the type
932-
type = this_val->as_Add()->add_ring(type, TypeInt::make(-this_adj_val))->is_int();
933-
}
934-
type = failtype->join(type)->is_int();
935-
if (type->empty()) {
936-
// Replace Bool with constant
937-
igvn->_worklist.push(in(1));
938-
igvn->replace_input_of(this, 1, igvn->intcon(proj_out(1-i)->_con));
939-
return true;
940-
}
941-
}
942-
}
943-
}
944-
}
945-
return false;
946-
}
947-
948840
// Check that the 2 CmpI can be folded into as single CmpU and proceed with the folding
949-
bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) {
841+
bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn) {
950842
Node* this_cmp = in(1)->in(1);
951843
BoolNode* this_bool = in(1)->as_Bool();
952844
IfNode* dom_iff = proj->in(0)->as_If();
@@ -955,17 +847,13 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail,
955847
Node* hi = this_cmp->in(2);
956848
Node* n = this_cmp->in(1);
957849
ProjNode* otherproj = proj->other_if_proj();
958-
assert(this_cmp->Opcode() == Op_CmpI && dom_iff->in(1)->in(1)->Opcode() == Op_CmpI, "Unexpected CmpNode");
850+
851+
const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj);
852+
const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success);
959853

960854
BoolTest::mask lo_test = dom_bool->_test._test;
961855
BoolTest::mask hi_test = this_bool->_test._test;
962856
BoolTest::mask cond = hi_test;
963-
if (lo_test == BoolTest::ne || hi_test == BoolTest::ne) {
964-
return false;
965-
}
966-
967-
const TypeInt* lo_type = IfNode::filtered_int_type(igvn, n, otherproj);
968-
const TypeInt* hi_type = IfNode::filtered_int_type(igvn, n, success);
969857

970858
// convert:
971859
//
@@ -993,7 +881,7 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail,
993881
// sets the lower bound if any.
994882
Node* adjusted_lim = NULL;
995883
if (lo_type != NULL && hi_type != NULL && hi_type->_lo > lo_type->_hi &&
996-
hi_type->_hi == max_jint && lo_type->_lo == min_jint) {
884+
hi_type->_hi == max_jint && lo_type->_lo == min_jint && lo_test != BoolTest::ne) {
997885
assert((dom_bool->_test.is_less() && !proj->_con) ||
998886
(dom_bool->_test.is_greater() && proj->_con), "incorrect test");
999887
// this test was canonicalized
@@ -1038,7 +926,7 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail,
1038926
return false;
1039927
}
1040928
} else if (lo_type != NULL && hi_type != NULL && lo_type->_lo > hi_type->_hi &&
1041-
lo_type->_hi == max_jint && hi_type->_lo == min_jint) {
929+
lo_type->_hi == max_jint && hi_type->_lo == min_jint && lo_test != BoolTest::ne) {
1042930

1043931
// this_bool = <
1044932
// dom_bool = < (proj = True) or dom_bool = >= (proj = False)
@@ -1096,6 +984,20 @@ bool IfNode::fold_to_unsigned(ProjNode* proj, ProjNode* success, ProjNode* fail,
1096984
return false;
1097985
}
1098986
} else {
987+
const TypeInt* failtype = filtered_int_type(igvn, n, proj);
988+
if (failtype != NULL) {
989+
const TypeInt* type2 = filtered_int_type(igvn, n, fail);
990+
if (type2 != NULL) {
991+
failtype = failtype->join(type2)->is_int();
992+
if (failtype->_lo > failtype->_hi) {
993+
// previous if determines the result of this if so
994+
// replace Bool with constant
995+
igvn->_worklist.push(in(1));
996+
igvn->replace_input_of(this, 1, igvn->intcon(success->_con));
997+
return true;
998+
}
999+
}
1000+
}
10991001
lo = NULL;
11001002
hi = NULL;
11011003
}
@@ -1359,57 +1261,42 @@ void IfNode::reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, Ph
13591261
Node* IfNode::fold_compares(PhaseIterGVN* igvn) {
13601262
if (Opcode() != Op_If) return NULL;
13611263

1362-
if (cmp_folds(igvn)) {
1264+
if (cmpi_folds(igvn)) {
13631265
Node* ctrl = in(0);
1364-
Node* cmp = in(1)->in(1);
1365-
Node* val = cmp->in(1);
1366-
// An integer comparison immediately dominated by another integer comparison
1367-
if (is_ctrl_folds(ctrl, igvn)) {
1368-
ProjNode* proj = ctrl->as_Proj();
1369-
if (fold_dominated_if(proj, igvn)) {
1266+
if (is_ctrl_folds(ctrl, igvn) && ctrl->outcnt() == 1) {
1267+
// A integer comparison immediately dominated by another integer
1268+
// comparison
1269+
ProjNode* success = NULL;
1270+
ProjNode* fail = NULL;
1271+
ProjNode* dom_cmp = ctrl->as_Proj();
1272+
if (has_shared_region(dom_cmp, success, fail) &&
1273+
// Next call modifies graph so must be last
1274+
fold_compares_helper(dom_cmp, success, fail, igvn)) {
13701275
return this;
13711276
}
1372-
Node* dom_cmp = ctrl->in(0)->in(1)->in(1);
1373-
Node* dom_val = dom_cmp->in(1);
1374-
if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val && ctrl->outcnt() == 1) {
1375-
ProjNode* success = NULL;
1376-
ProjNode* fail = NULL;
1377-
if (has_shared_region(proj, success, fail) &&
1378-
// Next call modifies graph so must be last
1379-
fold_to_unsigned(proj, success, fail, igvn)) {
1380-
return this;
1381-
}
1382-
if (has_only_uncommon_traps(proj, success, fail, igvn) &&
1383-
// Next call modifies graph so must be last
1384-
fold_to_unsigned(proj, success, fail, igvn)) {
1385-
return merge_uncommon_traps(proj, success, fail, igvn);
1386-
}
1277+
if (has_only_uncommon_traps(dom_cmp, success, fail, igvn) &&
1278+
// Next call modifies graph so must be last
1279+
fold_compares_helper(dom_cmp, success, fail, igvn)) {
1280+
return merge_uncommon_traps(dom_cmp, success, fail, igvn);
13871281
}
1388-
}
1389-
if (ctrl->in(0) != NULL &&
1390-
ctrl->in(0)->in(0) != NULL) {
1282+
return NULL;
1283+
} else if (ctrl->in(0) != NULL &&
1284+
ctrl->in(0)->in(0) != NULL) {
13911285
ProjNode* success = NULL;
13921286
ProjNode* fail = NULL;
13931287
Node* dom = ctrl->in(0)->in(0);
1394-
ProjNode* dom_proj = dom->isa_Proj();
1395-
ProjNode* other_proj = ctrl->isa_Proj();
1288+
ProjNode* dom_cmp = dom->isa_Proj();
1289+
ProjNode* other_cmp = ctrl->isa_Proj();
13961290

13971291
// Check if it's an integer comparison dominated by another
13981292
// integer comparison with another test in between
1399-
if (is_ctrl_folds(dom, igvn)) {
1400-
if (fold_dominated_if(dom_proj, igvn)) {
1401-
return this;
1402-
}
1403-
Node* dom_cmp = dom->in(0)->in(1)->in(1);
1404-
Node* dom_val = dom_cmp->in(1);
1405-
if (cmp->Opcode() == Op_CmpI && dom_cmp->Opcode() == Op_CmpI && val == dom_val &&
1406-
has_only_uncommon_traps(dom_proj, success, fail, igvn) &&
1407-
is_side_effect_free_test(other_proj, igvn) &&
1408-
// Next call modifies graph so must be last
1409-
fold_to_unsigned(dom_proj, success, fail, igvn)) {
1410-
reroute_side_effect_free_unc(other_proj, dom_proj, igvn);
1411-
return merge_uncommon_traps(dom_proj, success, fail, igvn);
1412-
}
1293+
if (is_ctrl_folds(dom, igvn) &&
1294+
has_only_uncommon_traps(dom_cmp, success, fail, igvn) &&
1295+
is_side_effect_free_test(other_cmp, igvn) &&
1296+
// Next call modifies graph so must be last
1297+
fold_compares_helper(dom_cmp, success, fail, igvn)) {
1298+
reroute_side_effect_free_unc(other_cmp, dom_cmp, igvn);
1299+
return merge_uncommon_traps(dom_cmp, success, fail, igvn);
14131300
}
14141301
}
14151302
}

src/hotspot/share/opto/parse2.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,10 +863,15 @@ bool Parse::create_jump_tables(Node* key_val, SwitchRange* lo, SwitchRange* hi)
863863

864864
// Clean the 32-bit int into a real 64-bit offset.
865865
// Otherwise, the jint value 0 might turn into an offset of 0x0800000000.
866-
const TypeInt* ikeytype = TypeInt::make(0, num_cases, Type::WidenMin);
867866
// Make I2L conversion control dependent to prevent it from
868867
// floating above the range check during loop optimizations.
869-
key_val = C->conv_I2X_index(&_gvn, key_val, ikeytype, control());
868+
// Do not use a narrow int type here to prevent the data path from dying
869+
// while the control path is not removed. This can happen if the type of key_val
870+
// is later known to be out of bounds of [0, num_cases] and therefore a narrow cast
871+
// would be replaced by TOP while C2 is not able to fold the corresponding range checks.
872+
#ifdef _LP64
873+
key_val = C->constrained_convI2L(&_gvn, key_val, TypeInt::INT, control());
874+
#endif
870875

871876
// Shift the value by wordsize so we have an index into the table, rather
872877
// than a switch value

0 commit comments

Comments
 (0)