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