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

[Chris Plummer]

It's probably the following:

void MacroAssembler::stop(const char* msg) {
   ExternalAddress message((address)msg);
   // push address of message
   pushptr(message.addr());
   { Label L; call(L, relocInfo::none); bind(L); }     // push eip
   pusha();                                            // push registers
   call(RuntimeAddress(CAST_FROM_FN_PTR(address, MacroAssembler::debug32)));
   hlt();
}

This is called for what are effectively asm asserts. There is no 
corresponding pop.

Chris

On 1/9/17 11:43 AM, Max Ockner wrote:
> Hi Chris,
> I may have misspoken. I was pretty convinced that there was a 
> push_CPU_state() call somewhere in one of the call_VM* methods since I 
> always see large blocks of register push and pop operations in the 
> debugger.  I checked the code and I couldn't find any such calls to 
> push_CPU_state().
> Max
>
> On 1/9/2017 12:14 PM, Chris Plummer wrote:
>> Hi Max,
>>
>> I took a quick look at the x86 call_VM() code and didn't see any 
>> register saving. Can you point me to it?
>>
>> thanks,
>>
>> Chris
>>
>> On 1/9/17 8:09 AM, Max Ockner wrote:
>>> Hi Chris,
>>>
>>> This looks good to me.
>>>
>>> As a side note, and from the perspective of someone who has been 
>>> digging in x86 rather than arm, I wonder if register saving can be 
>>> rolled into call_VM as is done in x86. It would certainly smooth 
>>> over some of the stylistic differences between the platforms.
>>>
>>> Thanks,
>>> Max
>>>
>>> On 1/6/2017 6: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
>>>>
>>>>
>>>
>>
>