RFR 8074717: Merge interp_masm files for x86 _32 and _64

Coleen Phillimore coleen.phillimore at oracle.com
Fri Mar 13 17:47:40 UTC 2015


On 3/13/15, 1:35 PM, harold seigel wrote:
> Hi Coleen,
>
> The changes look good but I have a few questions.
>
> 1. Why were rsi and rdi sometimes replaced with r13 and r14, and other 
> times with esi and edi, or bcp_register?
There was only that bit in the first comment so I fixed the comment and 
it makes sense now, looks like:

   // interpreter specific
   //
   // Note: No need to save/restore bcp & locals registers
   //       since these are callee saved registers and no blocking/
   //       GC can happen in leaf calls.
   // Further Note: DO NOT save/restore bcp/locals. If a caller has
   // already saved them so that it can use rsi/rdi as temporaries
   // then a save/restore here will DESTROY the copy the caller
   // saved! There used to be a save_bcp() that only happened in
   // the ASSERT path (no restore_bcp). Which caused bizarre failures
   // when jvm built with ASSERTs.


.. then later ...

   // interpreter specific
   // LP64: Used to ASSERT that r13/r14 were equal to frame's bcp/locals
   // but since they may not have been saved (and we don't want to
   // save them here (see note above) the assert is invalid.

It says "interpreter specific" because the compiled code also can call 
this call_VM_leaf_base function indirectly.
>
> 2. Do you plan to delete the "// XXX l ?" comments?

No, these were preexisting in one of the 64 or 32 bit versions.  I'm not 
going to change this because I don't know the answer...
>
> 3. How did you choose when to use long vs. ptr instructions?

In 32 bits sometimes there's a movptr which is the same as movl . In the 
64 bit version, the mov was movl which means that the data was actually 
32 bits and not a pointer.  So I chose movl.
>
> 4. At line 756 in interp_masm_x86_64.cpp, Why do the move here? Why 
> not just unlock_object(rmon) ?

On 32 bits robj and rmon are not the same registers and unlock_object 
has to have rdx.  On 64 bits the mov is a nop because the registers are 
the same and I don't think this affects any performance, so I had the 
mov be unconditional (without NOT_LP64).

>
> 5. At lines 814 and 1601 in interp_masm_x86_64.cpp, why /* */ instead 
> of //  for: #endif /* !CC_INTERP */

I fixed those.  The 32 bit and 64 bit files were inconsistent about this.

Thanks for going through this change!

Coleen
>
> Thanks, Harold
>
> On 3/9/2015 4:36 PM, Coleen Phillimore wrote:
>> Summary: Merge duplicate code and use #ifdef for non-mergeable sections.
>>
>> Inspired by Max's change and because I was working on this with him, 
>> I've merged interp_masm_x86_32 and _64.cpp/hpp files.
>>
>> There are two webrevs generated.  The first webrev is generated to 
>> see the diffs.   Mostly the _64 contents are chosen because the 
>> formatting was better.  The second is moving the code into the 
>> already existant interp_masm_x86.cpp/hpp files.
>>
>> Ran all jck tests on this and JPRT, also ran NSK vm.quick.testlist 
>> which includes jvmti tests.
>>
>> bug link https://bugs.openjdk.java.net/browse/JDK-8074717
>> open webrev at http://cr.openjdk.java.net/~coleenp/8074717
>> open webrev at http://cr.openjdk.java.net/~coleenp/8074717_2/
>>
>> Thanks,
>> Coleen
>



More information about the hotspot-dev mailing list