OpenJDK / valhalla / valhalla
changeset 47824:9ccaa4e79030
8190285: s390: Some java boolean checks are not correct
Reviewed-by: lucy, coleenp
author | mdoerr |
---|---|
date | Mon, 30 Oct 2017 17:14:39 +0100 |
parents | 1b0566927c7a |
children | 08fa85a36a67 |
files | src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp src/hotspot/cpu/s390/interp_masm_s390.cpp src/hotspot/cpu/s390/interp_masm_s390.hpp src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp src/hotspot/cpu/s390/templateTable_s390.cpp |
diffstat | 5 files changed, 68 insertions(+), 6 deletions(-) [+] |
line wrap: on
line diff
--- a/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp Sun Oct 29 18:13:18 2017 -0700 +++ b/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp Mon Oct 30 17:14:39 2017 +0100 @@ -277,7 +277,7 @@ length.set_instruction(x->length()); length.load_item(); } - if (needs_store_check) { + if (needs_store_check || x->check_boolean()) { value.load_item(); } else { value.load_for_store(x->elt_type()); @@ -327,11 +327,14 @@ // Needs GC write barriers. pre_barrier(LIR_OprFact::address(array_addr), LIR_OprFact::illegalOpr /* pre_val */, true /* do_load */, false /* patch */, NULL); - __ move(value.result(), array_addr, null_check_info); - // Seems to be a precise. + } + + LIR_Opr result = maybe_mask_boolean(x, array.result(), value.result(), null_check_info); + __ move(result, array_addr, null_check_info); + + if (obj_store) { + // Precise card mark post_barrier(LIR_OprFact::address(array_addr), value.result()); - } else { - __ move(value.result(), array_addr, null_check_info); } }
--- a/src/hotspot/cpu/s390/interp_masm_s390.cpp Sun Oct 29 18:13:18 2017 -0700 +++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp Mon Oct 30 17:14:39 2017 +0100 @@ -843,6 +843,38 @@ verify_oop(Z_tos, state); } +void InterpreterMacroAssembler::narrow(Register result, Register ret_type) { + get_method(ret_type); + z_lg(ret_type, Address(ret_type, in_bytes(Method::const_offset()))); + z_lb(ret_type, Address(ret_type, in_bytes(ConstMethod::result_type_offset()))); + + Label notBool, notByte, notChar, done; + + // common case first + compareU32_and_branch(ret_type, T_INT, bcondEqual, done); + + compareU32_and_branch(ret_type, T_BOOLEAN, bcondNotEqual, notBool); + z_nilf(result, 0x1); + z_bru(done); + + bind(notBool); + compareU32_and_branch(ret_type, T_BYTE, bcondNotEqual, notByte); + z_lbr(result, result); + z_bru(done); + + bind(notByte); + compareU32_and_branch(ret_type, T_CHAR, bcondNotEqual, notChar); + z_nilf(result, 0xffff); + z_bru(done); + + bind(notChar); + // compareU32_and_branch(ret_type, T_SHORT, bcondNotEqual, notShort); + z_lhr(result, result); + + // Nothing to do for T_INT + bind(done); +} + // remove activation // // Unlock the receiver if this is a synchronized method.
--- a/src/hotspot/cpu/s390/interp_masm_s390.hpp Sun Oct 29 18:13:18 2017 -0700 +++ b/src/hotspot/cpu/s390/interp_masm_s390.hpp Mon Oct 30 17:14:39 2017 +0100 @@ -86,6 +86,8 @@ void dispatch_next_noverify_oop(TosState state, int step = 0); void dispatch_via(TosState state, address* table); + void narrow(Register result, Register ret_type); + // Jump to an invoked target. void prepare_to_jump_from_interpreted(Register method); void jump_from_interpreted(Register method, Register temp);
--- a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp Sun Oct 29 18:13:18 2017 -0700 +++ b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp Mon Oct 30 17:14:39 2017 +0100 @@ -2377,6 +2377,12 @@ __ store_const(Address(RjvmtiState, JvmtiThreadState::earlyret_state_offset()), JvmtiThreadState::earlyret_inactive, 4, 4, Z_R0_scratch); + if (state == itos) { + // Narrow result if state is itos but result type is smaller. + // Need to narrow in the return bytecode rather than in generate_return_entry + // since compiled code callers expect the result to already be narrowed. + __ narrow(Z_tos, Z_tmp_1); /* fall through */ + } __ remove_activation(state, Z_tmp_1, // retaddr false, // throw_monitor_exception
--- a/src/hotspot/cpu/s390/templateTable_s390.cpp Sun Oct 29 18:13:18 2017 -0700 +++ b/src/hotspot/cpu/s390/templateTable_s390.cpp Mon Oct 30 17:14:39 2017 +0100 @@ -1174,8 +1174,20 @@ __ pop_i(Z_ARG3); __ pop_ptr(Z_tmp_2); // Z_tos : value - // Z_ARG3 : index + // Z_ARG3 : index // Z_tmp_2 : array + + // Need to check whether array is boolean or byte + // since both types share the bastore bytecode. + __ load_klass(Z_tmp_1, Z_tmp_2); + __ z_llgf(Z_tmp_1, Address(Z_tmp_1, Klass::layout_helper_offset())); + __ z_tmll(Z_tmp_1, Klass::layout_helper_boolean_diffbit()); + Label L_skip; + __ z_bfalse(L_skip); + // if it is a T_BOOLEAN array, mask the stored value to 0/1 + __ z_nilf(Z_tos, 0x1); + __ bind(L_skip); + // No index shift necessary - pass 0. index_check(Z_tmp_2, Z_ARG3, 0); // Prefer index in Z_ARG3. __ z_stc(Z_tos, @@ -2321,6 +2333,13 @@ __ bind(skip_register_finalizer); } + if (state == itos) { + // Narrow result if state is itos but result type is smaller. + // Need to narrow in the return bytecode rather than in generate_return_entry + // since compiled code callers expect the result to already be narrowed. + __ narrow(Z_tos, Z_tmp_1); /* fall through */ + } + __ remove_activation(state, Z_R14); __ z_br(Z_R14); }