RFR(S/T): 8230876: baseline cleanups from Async Monitor Deflation v2.0[789]
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Nov 19 20:11:50 UTC 2019
Vladimir,
Thanks for the review!
On 11/19/19 2:38 PM, Vladimir Kozlov wrote:
> macroAssembler_x86.cpp changes are only in comments which looks good
> to me.
Thanks.
> Fast_Lock/Fast_Unlock were names before I moved code from .ad files to
> macroAssembler_x86.cpp (8033805).
Yup. I figured we should match the current names.
> About "Without cast to int32_t this style of movptr will destroy r10
> which is typically obj."
> 64-bit version of movptr() has 2 versions of second argument, intptr_t
> and int32_t:
>
> http://hg.openjdk.java.net/jdk/jdk/file/faac483dfb30/src/hotspot/cpu/x86/macroAssembler_x86.cpp#l728
>
>
> intptr_t version use rscratch1 (R10) which is most likely is used and
> have valid value in it (in C2 generated code):
>
> http://hg.openjdk.java.net/jdk/jdk/file/faac483dfb30/src/hotspot/cpu/x86/x86_64.ad#l134
>
Thanks for confirmation that the comment is still valid.
Again, thanks for the review!
Dan
>
> Vladimir
>
> On 11/19/19 6:41 AM, Daniel D. Daugherty wrote:
>> Hi David,
>>
>> Thanks for the quick review!
>>
>> On 11/18/19 9:23 PM, David Holmes wrote:
>>> Hi Dan,
>>>
>>> Given:
>>>
>>> volatile intptr_t _recursions;
>>>
>>> The change to the print statements to use INTX_FORMAT instead of the
>>> existing INTPTR_FORMAT seems a little odd - but obviously you don't
>>> want it printed in hex.
>>
>> Yup. I first noticed it when I wrote and tested my initial version of
>> ObjectMonitor::print_debug_style_on() and then I realized that it was
>> all
>> over with other prints of that field.
>>
>>
>>> That seems fine, but can we then make the simple change to redefine
>>> _recursions as intx as well - which is a semantic no-op given:
>>>
>>> typedef intptr_t intx;
>>
>> No problem. I'll make that change before I push. Not sure why that
>> didn't occur to me before.
>>
>>
>>> Otherwise all seems okay.
>>
>> Thanks! Now, if we can just get a quick review of macroAssembler_x86.cpp
>> from Vladimir K... :-)
>>
>> Dan
>>
>>
>>>
>>> Thanks,
>>> David
>>>
>>> On 19/11/2019 8:23 am, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I have another round of baseline cleanup changes from the Async
>>>> Monitor
>>>> Deflation project (8153224). Like previous sub-tasks of 8153224, these
>>>> changes are small and/or trivial. These changes have previously been
>>>> reviewed as a (very) small part of 8153224 (CR8/v2.08/11-for-jdk14).
>>>>
>>>> Vladimir K., if you could sanity check the cleanups in
>>>> macroAssembler_x86.cpp
>>>> that would be appreciated (only comments were changed). I recommend
>>>> the
>>>> Udiff view...
>>>>
>>>> Please see the bug for details about the changes in this webrev:
>>>>
>>>> JDK-8230876 baseline cleanups from Async Monitor Deflation
>>>> v2.0[789]
>>>> https://bugs.openjdk.java.net/browse/JDK-8230876
>>>>
>>>> Here's the webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8230876-webrev/0-for-jdk14/
>>>>
>>>> These changes have been included in my recent rounds of Mach5
>>>> Tier[1-8]
>>>> and other associated stress and/or performance testing. I have also
>>>> done
>>>> a Mach5 Tier[1-3] run with just this patch to make sure that I got all
>>>> the pieces that are needed.
>>>>
>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>
>>>> Dan
>>
More information about the hotspot-runtime-dev
mailing list