# HG changeset patch # User johnc # Date 1366840123 25200 # Node ID d50cc62e94ff5a53de209fd9f9320a384beb9a57 # Parent 9d75bcd7c890ca64aadf667eab1dd8baea638299 8012715: G1: GraphKit accesses PtrQueue::_index as int but is size_t Summary: In graphKit INT operations were generated to access PtrQueue::_index which has type size_t. This is 64 bit on 64-bit machines. No problems occur on little endian machines as long as the index fits into 32 bit, but on big endian machines the upper part is read, which is zero. This leads to unnecessary branches to the slow path in the runtime. Reviewed-by: twisti, johnc Contributed-by: Martin Doerr diff -r 9d75bcd7c890 -r d50cc62e94ff src/share/vm/opto/graphKit.cpp --- a/src/share/vm/opto/graphKit.cpp Wed Apr 24 19:55:02 2013 +0200 +++ b/src/share/vm/opto/graphKit.cpp Wed Apr 24 14:48:43 2013 -0700 @@ -3564,7 +3564,8 @@ Node* no_ctrl = NULL; Node* no_base = __ top(); - Node* zero = __ ConI(0); + Node* zero = __ ConI(0); + Node* zeroX = __ ConX(0); float likely = PROB_LIKELY(0.999); float unlikely = PROB_UNLIKELY(0.999); @@ -3590,7 +3591,9 @@ // if (!marking) __ if_then(marking, BoolTest::ne, zero); { - Node* index = __ load(__ ctrl(), index_adr, TypeInt::INT, T_INT, Compile::AliasIdxRaw); + BasicType index_bt = TypeX_X->basic_type(); + assert(sizeof(size_t) == type2aelembytes(index_bt), "Loading G1 PtrQueue::_index with wrong size."); + Node* index = __ load(__ ctrl(), index_adr, TypeX_X, index_bt, Compile::AliasIdxRaw); if (do_load) { // load original value @@ -3603,22 +3606,16 @@ Node* buffer = __ load(__ ctrl(), buffer_adr, TypeRawPtr::NOTNULL, T_ADDRESS, Compile::AliasIdxRaw); // is the queue for this thread full? - __ if_then(index, BoolTest::ne, zero, likely); { + __ if_then(index, BoolTest::ne, zeroX, likely); { // decrement the index - Node* next_index = __ SubI(index, __ ConI(sizeof(intptr_t))); - Node* next_indexX = next_index; -#ifdef _LP64 - // We could refine the type for what it's worth - // const TypeLong* lidxtype = TypeLong::make(CONST64(0), get_size_from_queue); - next_indexX = _gvn.transform( new (C) ConvI2LNode(next_index, TypeLong::make(0, max_jlong, Type::WidenMax)) ); -#endif + Node* next_index = _gvn.transform(new (C) SubXNode(index, __ ConX(sizeof(intptr_t)))); // Now get the buffer location we will log the previous value into and store it - Node *log_addr = __ AddP(no_base, buffer, next_indexX); + Node *log_addr = __ AddP(no_base, buffer, next_index); __ store(__ ctrl(), log_addr, pre_val, T_OBJECT, Compile::AliasIdxRaw); // update the index - __ store(__ ctrl(), index_adr, next_index, T_INT, Compile::AliasIdxRaw); + __ store(__ ctrl(), index_adr, next_index, index_bt, Compile::AliasIdxRaw); } __ else_(); { @@ -3645,26 +3642,21 @@ Node* buffer, const TypeFunc* tf) { - Node* zero = __ ConI(0); + Node* zero = __ ConI(0); + Node* zeroX = __ ConX(0); Node* no_base = __ top(); BasicType card_bt = T_BYTE; // Smash zero into card. MUST BE ORDERED WRT TO STORE __ storeCM(__ ctrl(), card_adr, zero, oop_store, oop_alias_idx, card_bt, Compile::AliasIdxRaw); // Now do the queue work - __ if_then(index, BoolTest::ne, zero); { - - Node* next_index = __ SubI(index, __ ConI(sizeof(intptr_t))); - Node* next_indexX = next_index; -#ifdef _LP64 - // We could refine the type for what it's worth - // const TypeLong* lidxtype = TypeLong::make(CONST64(0), get_size_from_queue); - next_indexX = _gvn.transform( new (C) ConvI2LNode(next_index, TypeLong::make(0, max_jlong, Type::WidenMax)) ); -#endif // _LP64 - Node* log_addr = __ AddP(no_base, buffer, next_indexX); + __ if_then(index, BoolTest::ne, zeroX); { + + Node* next_index = _gvn.transform(new (C) SubXNode(index, __ ConX(sizeof(intptr_t)))); + Node* log_addr = __ AddP(no_base, buffer, next_index); __ store(__ ctrl(), log_addr, card_adr, T_ADDRESS, Compile::AliasIdxRaw); - __ store(__ ctrl(), index_adr, next_index, T_INT, Compile::AliasIdxRaw); + __ store(__ ctrl(), index_adr, next_index, TypeX_X->basic_type(), Compile::AliasIdxRaw); } __ else_(); { __ make_leaf_call(tf, CAST_FROM_FN_PTR(address, SharedRuntime::g1_wb_post), "g1_wb_post", card_adr, __ thread()); @@ -3725,7 +3717,7 @@ // Now some values // Use ctrl to avoid hoisting these values past a safepoint, which could // potentially reset these fields in the JavaThread. - Node* index = __ load(__ ctrl(), index_adr, TypeInt::INT, T_INT, Compile::AliasIdxRaw); + Node* index = __ load(__ ctrl(), index_adr, TypeX_X, TypeX_X->basic_type(), Compile::AliasIdxRaw); Node* buffer = __ load(__ ctrl(), buffer_adr, TypeRawPtr::NOTNULL, T_ADDRESS, Compile::AliasIdxRaw); // Convert the store obj pointer to an int prior to doing math on it