# HG changeset patch # User mdoerr # Date 1509113097 -7200 # Fri Oct 27 16:04:57 2017 +0200 # Node ID 597e3866be2a65689d8474d32cf94e7d0fd27e27 # Parent c15f15bcc23efe256fdc97a2e5fbebea9d7158d4 8190285: s390: Some java boolean checks are not correct Reviewed-by: diff --git a/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp b/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp --- a/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp +++ b/src/hotspot/cpu/s390/c1_LIRGenerator_s390.cpp @@ -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); } } diff --git a/src/hotspot/cpu/s390/interp_masm_s390.cpp b/src/hotspot/cpu/s390/interp_masm_s390.cpp --- a/src/hotspot/cpu/s390/interp_masm_s390.cpp +++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp @@ -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. diff --git a/src/hotspot/cpu/s390/interp_masm_s390.hpp b/src/hotspot/cpu/s390/interp_masm_s390.hpp --- a/src/hotspot/cpu/s390/interp_masm_s390.hpp +++ b/src/hotspot/cpu/s390/interp_masm_s390.hpp @@ -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); diff --git a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp --- a/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp +++ b/src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp @@ -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 diff --git a/src/hotspot/cpu/s390/templateTable_s390.cpp b/src/hotspot/cpu/s390/templateTable_s390.cpp --- a/src/hotspot/cpu/s390/templateTable_s390.cpp +++ b/src/hotspot/cpu/s390/templateTable_s390.cpp @@ -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); }