[aarch64-port-dev ] RFR: 8075930: AARCH64: Use FP Register in C2

[Vladimir Kozlov]

Hi Edward,

In frame() I think check !defined(COMPILER2) is useless because COMPILER2 is always defined in 64-bit Server Hotspot VM. 
Unless you have configuration without C2. I think it should be dynamic check after you find who generated this _cb.
Can you explain what this code does (there is not comment):

   80 #ifndef PRODUCT
   81   if (fp)
   82     if (sp > fp || (fp - sp > 0x100000))
   83       for(;;)
   84         asm("nop");
   85 #endif

Thanks,
Vladimir

On 4/9/15 2:31 AM, Edward Nevill wrote:
> Hi Zoltán,
>
> Thanks for the review. I have updated the patch to take account of your comments. New patch at
>
> http://cr.openjdk.java.net/~enevill/8075930/webrev.02/
>
> Could someone please review this patch and if approved push.
>
> See inline comments below.
>
> On Wed, 2015-03-25 at 14:28 +0100, Zoltán Majó wrote:
>> On 03/25/2015 02:02 PM, Edward Nevill wrote:
>>> Hi,
>>>
>>> The aarch64 C2 compiler makes no use of the FP register even though this register is saved on entry and restored on exit and is therefore always available for allocation.
>>
>> It seems to me that the C2 compiler currently saves the SP into FP on
>> method handle invokes. This functionality is encoded by
>> aarch64_enc_java_handle_call in aarch64.ad. C1 does the same (see usages
>> of FrameMap::method_handle_invoke_SP_save_opr()).
>
> So, this is an issue for C2 only, since my proposed patch does not affect register usage in C1, so C1 will continue to keep FP as unallocated.
>
> The change required for C2 is to let the register allocator know that FP will be killed by a methodhandle call. This is accomplished by the following change in aarch64.ad
>
>   const RegMask Matcher::method_handle_invoke_SP_save_mask() {
> -  return RegMask();
> +  return FP_REG_mask();
>   }
>
>>
>> Stack walking relies on having the SP saved into FP for method handle
>> invokes (e.g., in frame::adjust_unextended_sp()).
>>
>> Saving the SP into FP is not needed on some architectures (e.g., x86).
>> I'm currently working on an enhancement that disables saving the SP into
>> FP on x86 (see JDK-8068945). Maybe that could work for aarch64 as well.
>>
>> Does your change consider this aspect? Maybe running the tests in
>> jdk/test/java/lang/invoke/ could trigger some failures that are caused
>> by the FP not containing the SP of the caller of a method handle invocation.
>>
>
> I have run the jdk/test/java/lang/invoke tests both with and without the
> proposed patch. In bot cases 62 tests out of 62 passed.
>
> I have also rerun JTreg hotspot and langtools with the following
> results:-
>
> Before:-
>
> Hotspot: Test results: passed: 596; failed: 190; error: 26
> Langtools: Test results: passed: 3,237; error: 9
>
> After:-
>
> Hotspot: Test results: passed: 596; failed: 190; error: 26
> Langtools: Test results: passed: 3,238; error: 8
>
> All the best,
> Ed.
>
>
>
>