Request for review (XXS): -XX:+TraceBytecodes does not work with -XX:+UseCompressedOops
Volker Simonis
volker.simonis at gmail.com
Fri Dec 16 15:57:35 PST 2011
Thank you Coleen.
Regards,
Volker
On Fri, Dec 16, 2011 at 7:00 PM, Coleen Phillimore
<coleen.phillimore at oracle.com> wrote:
> Volker, Thank you for fixing this. It looks good to me. I can check it in
> this contribution, pending additional comments.
>
> Coleen
>
>
> On 12/16/2011 12:20 PM, Volker Simonis wrote:
>>
>> Hi,
>>
>> I've just realized that -XX:+TraceBytecodes does not work anymore if
>> -XX:+UseCompressedOops is set.
>>
>> This is because TemplateInterpreterGenerator::trace_bytecode() uses
>> 'r12' which holds the heap base for temporary saving the 'sp' before
>> calling the Interpreter::trace_code() template:
>>
>> __ mov(r12, rsp); // remember sp (can only use r12 if not using call_VM)
>> __ andptr(rsp, -16); // align stack as required by ABI
>> __ call(RuntimeAddress(Interpreter::trace_code(t->tos_in())));
>> __ mov(rsp, r12); // restore sp
>> __ reinit_heapbase();
>>
>> Notice that the comment explicitly mentions that 'r12' can only be
>> used if 'call_VM' will not be used subsequently.
>>
>> But this is exactly what Interpreter::trace_code() does:
>>
>>
>> address TemplateInterpreterGenerator::generate_trace_code(TosState state)
>> {
>> ...
>> __ call_VM(noreg,...
>>
>> void MacroAssembler::call_VM(Register oop_result,
>> ...
>> call_VM_helper(oop_result, entry_point, 3, check_exceptions);
>>
>> void MacroAssembler::call_VM_helper(Register oop_result, address
>> entry_point, int number_of_arguments, bool check_exceptions) {
>> ...
>> call_VM_base(oop_result, noreg, rax, entry_point,
>> number_of_arguments, check_exceptions);
>>
>> void MacroAssembler::call_VM_base(Register oop_result,
>> ...
>> LP64_ONLY(if (UseCompressedOops) verify_heapbase("call_VM_base");)
>>
>>
>> This finally leads to a call to 'verify_heapbase()' which will fail
>> because 'r12' has been overwritten.
>>
>> The proposed fix is trivial: don't do the check if we are running with
>> -XX:+TraceBytecodes:
>>
>> diff -r 86cbe939f0c7 src/cpu/x86/vm/assembler_x86.cpp
>> --- a/src/cpu/x86/vm/assembler_x86.cpp Mon Sep 19 12:18:46 2011 -0700
>> +++ b/src/cpu/x86/vm/assembler_x86.cpp Fri Dec 16 18:01:31 2011 +0100
>> @@ -5967,7 +5967,7 @@
>> assert(number_of_arguments>= 0 , "cannot have negative number of
>> arguments");
>> LP64_ONLY(assert(java_thread == r15_thread, "unexpected register"));
>> #ifdef ASSERT
>> - LP64_ONLY(if (UseCompressedOops) verify_heapbase("call_VM_base");)
>> + LP64_ONLY(if (UseCompressedOops&& !TraceBytecodes)
>>
>> verify_heapbase("call_VM_base");)
>> #endif // ASSERT
>>
>> assert(java_thread != oop_result , "cannot use the same register
>> for java_thread& oop_result");
>>
>>
>> For your convenience I've also attached the patch as a file to this
>> message.
>>
>> Regards,
>> Volker
More information about the hotspot-runtime-dev
mailing list