Skip to content

Commit

Permalink
8295407: C2 crash: Error: ShouldNotReachHere() in multiple vector tes…
Browse files Browse the repository at this point in the history
…ts with -XX:-MonomorphicArrayCheck -XX:-UncommonNullCast

Reviewed-by: thartmann, kvn
  • Loading branch information
Fei Gao authored and Pengfei Li committed Nov 17, 2022
1 parent e2269fd commit cc44419
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 102 deletions.
147 changes: 58 additions & 89 deletions src/hotspot/share/opto/superword.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ bool SuperWord::SLP_extract() {

construct_my_pack_map();
if (UseVectorCmov) {
merge_packs_to_cmovd();
merge_packs_to_cmove();
}

filter_packs();
Expand Down Expand Up @@ -1411,7 +1411,7 @@ int SuperWord::data_size(Node* s) {
if (use != NULL) {
return data_size(use);
}
use = _cmovev_kit.is_CmpD_candidate(s);
use = _cmovev_kit.is_Cmp_candidate(s);
if (use != NULL) {
return data_size(use);
}
Expand Down Expand Up @@ -1845,33 +1845,19 @@ void SuperWord::filter_packs() {
#endif
}

// Clear the unused cmove pack and its related packs from superword candidate packset.
void SuperWord::remove_cmove_and_related_packs(Node_List* cmove_pk) {
Node* cmove = cmove_pk->at(0);
Node* bol = cmove->as_CMove()->in(CMoveNode::Condition);
if (my_pack(bol)) {
remove_pack(my_pack(bol));
}
Node* cmp = bol->in(1);
if (my_pack(cmp)) {
remove_pack(my_pack(cmp));
}
remove_pack(cmove_pk);
}

//------------------------------merge_packs_to_cmovd---------------------------
// Merge qualified CMoveD into new vector-nodes
// We want to catch this pattern and subsume CmpD and Bool into CMoveD
//------------------------------merge_packs_to_cmove---------------------------
// Merge qualified CMove into new vector-nodes
// We want to catch this pattern and subsume Cmp and Bool into CMove
//
// SubD ConD
// Sub Con
// / | /
// / | / /
// / | / /
// / | / /
// / / /
// / / | /
// v / | /
// CmpD | /
// Cmp | /
// | | /
// v | /
// Bool | /
Expand All @@ -1880,25 +1866,20 @@ void SuperWord::remove_cmove_and_related_packs(Node_List* cmove_pk) {
// \ | /
// \ | /
// \ v /
// CMoveD
// CMove
//
// Also delete unqualified CMove pack from the packset and clear all info.

void SuperWord::merge_packs_to_cmovd() {
void SuperWord::merge_packs_to_cmove() {
for (int i = _packset.length() - 1; i >= 0; i--) {
Node_List* pk = _packset.at(i);
if (_cmovev_kit.is_cmove_pack_candidate(pk)) {
if (_cmovev_kit.can_merge_cmove_pack(pk)) {
_cmovev_kit.make_cmove_pack(pk);
} else {
remove_cmove_and_related_packs(pk);
}
if (_cmovev_kit.can_merge_cmove_pack(pk)) {
_cmovev_kit.make_cmove_pack(pk);
}
}

#ifndef PRODUCT
if (TraceSuperWord) {
tty->print_cr("\nSuperWord::merge_packs_to_cmovd(): After merge");
tty->print_cr("\nSuperWord::merge_packs_to_cmove(): After merge");
print_packset();
tty->cr();
}
Expand All @@ -1919,7 +1900,7 @@ Node* CMoveKit::is_Bool_candidate(Node* def) const {
return use;
}

Node* CMoveKit::is_CmpD_candidate(Node* def) const {
Node* CMoveKit::is_Cmp_candidate(Node* def) const {
Node* use = NULL;
if (!def->is_Cmp() || def->in(0) != NULL || def->outcnt() != 1) {
return NULL;
Expand All @@ -1933,32 +1914,28 @@ Node* CMoveKit::is_CmpD_candidate(Node* def) const {
return use;
}

bool CMoveKit::is_cmove_pack_candidate(Node_List* cmove_pk) {
Node* cmove = cmove_pk->at(0);
if ((cmove->Opcode() != Op_CMoveF && cmove->Opcode() != Op_CMoveD) ||
pack(cmove) != NULL /* already in the cmove pack */) {
return false;
}
return true;
}

// Determine if the current pack is an ideal cmove pack, and if its related packs,
// i.e. bool node pack and cmp node pack, can be successfully merged for vectorization.
bool CMoveKit::can_merge_cmove_pack(Node_List* cmove_pk) {
Node* cmove = cmove_pk->at(0);

if (!SuperWord::is_cmove_fp_opcode(cmove->Opcode()) ||
pack(cmove) != NULL /* already in the cmove pack */) {
return false;
}

if (cmove->in(0) != NULL) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::make_cmove_pack: CMove %d has control flow, escaping...", cmove->_idx); cmove->dump();})
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::can_merge_cmove_pack: CMove %d has control flow, escaping...", cmove->_idx); cmove->dump();})
return false;
}

Node* bol = cmove->as_CMove()->in(CMoveNode::Condition);
if (!bol->is_Bool() ||
bol->outcnt() != 1 ||
!_sw->same_generation(bol, cmove) ||
bol->in(0) != NULL || // BoolNode has control flow!!
bol->in(0) != NULL || // Bool node has control flow!!
_sw->my_pack(bol) == NULL) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::make_cmove_pack: Bool %d does not fit CMove %d for building vector, escaping...", bol->_idx, cmove->_idx); bol->dump();})
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::can_merge_cmove_pack: Bool %d does not fit CMove %d for building vector, escaping...", bol->_idx, cmove->_idx); bol->dump();})
return false;
}
Node_List* bool_pk = _sw->my_pack(bol);
Expand All @@ -1970,18 +1947,18 @@ bool CMoveKit::can_merge_cmove_pack(Node_List* cmove_pk) {
if (!cmp->is_Cmp() ||
cmp->outcnt() != 1 ||
!_sw->same_generation(cmp, cmove) ||
cmp->in(0) != NULL || // CmpNode has control flow!!
cmp->in(0) != NULL || // Cmp node has control flow!!
_sw->my_pack(cmp) == NULL) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::make_cmove_pack: Cmp %d does not fit CMove %d for building vector, escaping...", cmp->_idx, cmove->_idx); cmp->dump();})
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::can_merge_cmove_pack: Cmp %d does not fit CMove %d for building vector, escaping...", cmp->_idx, cmove->_idx); cmp->dump();})
return false;
}
Node_List* cmp_pk = _sw->my_pack(cmp);
if (cmp_pk->size() != cmove_pk->size() ) {
return false;
}

if (!test_cmpd_pack(cmp_pk, cmove_pk)) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::make_cmove_pack: cmp pack for Cmp %d failed vectorization test", cmp->_idx); cmp->dump();})
if (!test_cmp_pack(cmp_pk, cmove_pk)) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print("CMoveKit::can_merge_cmove_pack: cmp pack for Cmp %d failed vectorization test", cmp->_idx); cmp->dump();})
return false;
}

Expand Down Expand Up @@ -2019,59 +1996,59 @@ void CMoveKit::make_cmove_pack(Node_List* cmove_pk) {
NOT_PRODUCT(if(_sw->is_trace_cmov()) {tty->print_cr("CMoveKit::make_cmove_pack: added syntactic CMove pack"); _sw->print_pack(new_cmove_pk);})
}

bool CMoveKit::test_cmpd_pack(Node_List* cmpd_pk, Node_List* cmovd_pk) {
Node* cmpd0 = cmpd_pk->at(0);
assert(cmpd0->is_Cmp(), "CMoveKit::test_cmpd_pack: should be CmpDNode");
assert(cmovd_pk->at(0)->is_CMove(), "CMoveKit::test_cmpd_pack: should be CMoveD");
assert(cmpd_pk->size() == cmovd_pk->size(), "CMoveKit::test_cmpd_pack: should be same size");
Node* in1 = cmpd0->in(1);
Node* in2 = cmpd0->in(2);
bool CMoveKit::test_cmp_pack(Node_List* cmp_pk, Node_List* cmove_pk) {
Node* cmp0 = cmp_pk->at(0);
assert(cmp0->is_Cmp(), "CMoveKit::test_cmp_pack: should be Cmp Node");
assert(cmove_pk->at(0)->is_CMove(), "CMoveKit::test_cmp_pack: should be CMove");
assert(cmp_pk->size() == cmove_pk->size(), "CMoveKit::test_cmp_pack: should be same size");
Node* in1 = cmp0->in(1);
Node* in2 = cmp0->in(2);
Node_List* in1_pk = _sw->my_pack(in1);
Node_List* in2_pk = _sw->my_pack(in2);

if ( (in1_pk != NULL && in1_pk->size() != cmpd_pk->size())
|| (in2_pk != NULL && in2_pk->size() != cmpd_pk->size()) ) {
if ( (in1_pk != NULL && in1_pk->size() != cmp_pk->size())
|| (in2_pk != NULL && in2_pk->size() != cmp_pk->size()) ) {
return false;
}

// test if "all" in1 are in the same pack or the same node
if (in1_pk == NULL) {
for (uint j = 1; j < cmpd_pk->size(); j++) {
if (cmpd_pk->at(j)->in(1) != in1) {
for (uint j = 1; j < cmp_pk->size(); j++) {
if (cmp_pk->at(j)->in(1) != in1) {
return false;
}
}//for: in1_pk is not pack but all CmpD nodes in the pack have the same in(1)
}//for: in1_pk is not pack but all Cmp nodes in the pack have the same in(1)
}
// test if "all" in2 are in the same pack or the same node
if (in2_pk == NULL) {
for (uint j = 1; j < cmpd_pk->size(); j++) {
if (cmpd_pk->at(j)->in(2) != in2) {
for (uint j = 1; j < cmp_pk->size(); j++) {
if (cmp_pk->at(j)->in(2) != in2) {
return false;
}
}//for: in2_pk is not pack but all CmpD nodes in the pack have the same in(2)
}
//now check if cmpd_pk may be subsumed in vector built for cmovd_pk
int cmovd_ind1, cmovd_ind2;
if (cmpd_pk->at(0)->in(1) == cmovd_pk->at(0)->as_CMove()->in(CMoveNode::IfFalse)
&& cmpd_pk->at(0)->in(2) == cmovd_pk->at(0)->as_CMove()->in(CMoveNode::IfTrue)) {
cmovd_ind1 = CMoveNode::IfFalse;
cmovd_ind2 = CMoveNode::IfTrue;
} else if (cmpd_pk->at(0)->in(2) == cmovd_pk->at(0)->as_CMove()->in(CMoveNode::IfFalse)
&& cmpd_pk->at(0)->in(1) == cmovd_pk->at(0)->as_CMove()->in(CMoveNode::IfTrue)) {
cmovd_ind2 = CMoveNode::IfFalse;
cmovd_ind1 = CMoveNode::IfTrue;
}//for: in2_pk is not pack but all Cmp nodes in the pack have the same in(2)
}
//now check if cmp_pk may be subsumed in vector built for cmove_pk
int cmove_ind1, cmove_ind2;
if (cmp_pk->at(0)->in(1) == cmove_pk->at(0)->as_CMove()->in(CMoveNode::IfFalse)
&& cmp_pk->at(0)->in(2) == cmove_pk->at(0)->as_CMove()->in(CMoveNode::IfTrue)) {
cmove_ind1 = CMoveNode::IfFalse;
cmove_ind2 = CMoveNode::IfTrue;
} else if (cmp_pk->at(0)->in(2) == cmove_pk->at(0)->as_CMove()->in(CMoveNode::IfFalse)
&& cmp_pk->at(0)->in(1) == cmove_pk->at(0)->as_CMove()->in(CMoveNode::IfTrue)) {
cmove_ind2 = CMoveNode::IfFalse;
cmove_ind1 = CMoveNode::IfTrue;
}
else {
return false;
}

for (uint j = 1; j < cmpd_pk->size(); j++) {
if (cmpd_pk->at(j)->in(1) != cmovd_pk->at(j)->as_CMove()->in(cmovd_ind1)
|| cmpd_pk->at(j)->in(2) != cmovd_pk->at(j)->as_CMove()->in(cmovd_ind2)) {
for (uint j = 1; j < cmp_pk->size(); j++) {
if (cmp_pk->at(j)->in(1) != cmove_pk->at(j)->as_CMove()->in(cmove_ind1)
|| cmp_pk->at(j)->in(2) != cmove_pk->at(j)->as_CMove()->in(cmove_ind2)) {
return false;
}//if
}
NOT_PRODUCT(if(_sw->is_trace_cmov()) { tty->print("CMoveKit::test_cmpd_pack: cmpd pack for 1st CmpD %d is OK for vectorization: ", cmpd0->_idx); cmpd0->dump(); })
NOT_PRODUCT(if(_sw->is_trace_cmov()) { tty->print("CMoveKit::test_cmp_pack: cmp pack for 1st Cmp %d is OK for vectorization: ", cmp0->_idx); cmp0->dump(); })
return true;
}

Expand Down Expand Up @@ -2099,6 +2076,9 @@ bool SuperWord::implemented(Node_List* p) {
// integer subword types with superword vectorization.
// See JDK-8294816 for miscompilation issues with shorts.
return false;
} else if (is_cmove_fp_opcode(opc)) {
retValue = is_cmov_pack(p) && VectorNode::implemented(opc, size, velt_basic_type(p0));
NOT_PRODUCT(if(retValue && is_trace_cmov()) {tty->print_cr("SWPointer::implemented: found cmove pack"); print_pack(p);})
} else {
// Vector unsigned right shift for signed subword types behaves differently
// from Java Spec. But when the shift amount is a constant not greater than
Expand All @@ -2108,7 +2088,6 @@ bool SuperWord::implemented(Node_List* p) {
opc = Op_RShiftI;
}
retValue = VectorNode::implemented(opc, size, velt_basic_type(p0));
NOT_PRODUCT(if(retValue && is_trace_cmov() && is_cmov_pack(p)) {tty->print_cr("SWPointer::implemented: found cmpd pack"); print_pack(p);})
}
}
return retValue;
Expand Down Expand Up @@ -3719,16 +3698,6 @@ void SuperWord::remove_pack_at(int pos) {
_packset.remove_at(pos);
}

//------------------------------remove_pack------------------------------
// Remove the pack in the packset
void SuperWord::remove_pack(Node_List* p) {
for (uint i = 0; i < p->size(); i++) {
Node* s = p->at(i);
set_my_pack(s, NULL);
}
_packset.remove(p);
}

void SuperWord::packset_sort(int n) {
// simple bubble sort so that we capitalize with O(n) when its already sorted
while (n != 0) {
Expand Down
21 changes: 8 additions & 13 deletions src/hotspot/share/opto/superword.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,11 @@ class CMoveKit {
void unmap(Node* key) { _dict->Delete(_2p(key)); }
Node_List* pack(Node* key) const { return (Node_List*)_dict->operator[](_2p(key)); }
Node* is_Bool_candidate(Node* nd) const; // if it is the right candidate return corresponding CMove* ,
Node* is_CmpD_candidate(Node* nd) const; // otherwise return NULL
// If the input pack is a cmove candidate, return true, otherwise return false.
bool is_cmove_pack_candidate(Node_List* cmove_pk);
// Determine if the current cmove pack can be vectorized.
Node* is_Cmp_candidate(Node* nd) const; // otherwise return NULL
// Determine if the current pack is a cmove candidate that can be vectorized.
bool can_merge_cmove_pack(Node_List* cmove_pk);
void make_cmove_pack(Node_List* cmovd_pk);
bool test_cmpd_pack(Node_List* cmpd_pk, Node_List* cmovd_pk);
void make_cmove_pack(Node_List* cmove_pk);
bool test_cmp_pack(Node_List* cmp_pk, Node_List* cmove_pk);
};//class CMoveKit

// JVMCI: OrderedPair is moved up to deal with compilation issues on Windows
Expand Down Expand Up @@ -455,9 +453,10 @@ class SuperWord : public ResourceObj {
// my_pack
Node_List* my_pack(Node* n) { return !in_bb(n) ? NULL : _node_info.adr_at(bb_idx(n))->_my_pack; }
void set_my_pack(Node* n, Node_List* p) { int i = bb_idx(n); grow_node_info(i); _node_info.adr_at(i)->_my_pack = p; }
// is pack good for converting into one vector node replacing 12 nodes of Cmp, Bool, CMov
// is pack good for converting into one vector node replacing bunches of Cmp, Bool, CMov nodes.
bool is_cmov_pack(Node_List* p);
bool is_cmov_pack_internal_node(Node_List* p, Node* nd) { return is_cmov_pack(p) && !nd->is_CMove(); }
static bool is_cmove_fp_opcode(int opc) { return (opc == Op_CMoveF || opc == Op_CMoveD); }
// For pack p, are all idx operands the same?
bool same_inputs(Node_List* p, int idx);
// CloneMap utilities
Expand Down Expand Up @@ -540,10 +539,8 @@ class SuperWord : public ResourceObj {
void construct_my_pack_map();
// Remove packs that are not implemented or not profitable.
void filter_packs();
// Clear the unused cmove pack and its related packs from superword candidate packset.
void remove_cmove_and_related_packs(Node_List* cmove_pk);
// Merge CMoveD into new vector-nodes
void merge_packs_to_cmovd();
// Merge CMove into new vector-nodes
void merge_packs_to_cmove();
// Adjust the memory graph for the packed operations
void schedule();
// Remove "current" from its current position in the memory graph and insert
Expand Down Expand Up @@ -590,8 +587,6 @@ class SuperWord : public ResourceObj {
Node_List* in_pack(Node* s, Node_List* p);
// Remove the pack at position pos in the packset
void remove_pack_at(int pos);
// Remove the pack in the packset
void remove_pack(Node_List* p);
// Return the node executed first in pack p.
Node* executed_first(Node_List* p);
// Return the node executed last in pack p.
Expand Down

1 comment on commit cc44419

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.