RFR(M) 8155980: ARM InterpreterMacroAssembler::get_method_counters() should not be saving callee saved registers

Chris Plummer chris.plummer at oracle.com
Mon Jan 9 17:08:47 UTC 2017


Thanks Dean. Unfortunately I got a crash over the weekend when 
repeatedly running one of our test suites. Happens about 1 out of 10 
times on both 32-bit and 64-bit. Looking into it now.

Chris

On 1/6/17 3:57 PM, dean.long at oracle.com wrote:
> It looks good but you might want to add a comment in 
> TemplateTable::branch() saying something like:
>
> // save and restore caller-saved registers that will be trashed by 
> call_VM
>
> dl
>
>
> On 1/6/17 3:14 PM, Chris Plummer wrote:
>> Hello,
>>
>> Please review the following arm changes. They will be pushed to JDK 
>> 10 only (when it opens).
>>
>> https://bugs.openjdk.java.net/browse/JDK-8155980
>> http://cr.openjdk.java.net/~cjplummer/8155980/webrev.02/webrev.hotspot
>>
>> JDK-8012544 added a bunch of callee register saving to 
>> InterpreterMacroAssembler::get_method_counters() around the call_VM() 
>> call. The reason is because there are a couple of callers of 
>> get_method_counters() that need to have r0 and r3 preserved. 
>> get_method_counters() should not be responsible for having to save 
>> callee saved registers. It does not know which ones are in use when 
>> it is called, so it can't do this efficiently. In fact 2 of the 4 
>> callers of get_method_counters() do not need any registers saved. For 
>> this reason the callee of get_method_counters() should be responsible 
>> for saving callee registers.
>>
>> This solution would have been been pretty straight forward, except 
>> that AARCH64 does not allow SP to go above extended_sp, so when 
>> setting up extended_sp I needed to make sure there will be room for 
>> the 2 registers that need to be pushed. extended_sp is mainly based 
>> on max_stack() for the method, plus an extra slot for jsr292 and 
>> exception handling (but not both at the same time). So the fix here 
>> is mostly about making sure there are always at least 2 extra slots 
>> for pushing the 2 registers.
>>
>> Here are the changes:
>>
>> interp_masm_arm.cpp
>>   -No longer save/restore registers in get_method_counters():
>>
>> templateTable_arm.cpp:
>>   -Save/restore Rdisp and R3_bytecode to stack around calls to 
>> get_method_counters.
>>
>> abstractinterpreter__arm.cpp::
>>   -Properly account for extra 2 slots needed on AARCH64 when creating 
>> a frame
>>    in layout_activation()
>>   -Note I switched to using method->constMethod()->max_stack() because
>>    method->max_stack() includes Method::extra_stack_entries(), and I 
>> want to
>>    account for Method::extra_stack_entries() separately.
>>
>> templateInterpreterGenerator_arm.cpp:
>>   -Properly account for extra 2 slots needed on AARCH64 when creating 
>> a frame
>>    in generate_normal_entry().
>>
>> I've tested the following with -Xcomp and with -Xmixed on arm32 and 
>> arm64:
>>
>>   Kitchensink
>>   vm.mlvm
>>   :jdk_svc
>>   :hotspot_runtime
>>   :hotspot_serviceability
>>   :hotspot_compiler
>>   :jdk_lang
>>   :jdk_util
>>
>> I also tested the test case from JDK-8012544.
>>
>> thanks,
>>
>> Chris
>>
>>
>



More information about the hotspot-runtime-dev mailing list