[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR (T) 8250042: Clean up methodOop and method_oop names from the code
From:       coleen.phillimore () oracle ! com
Date:       2020-07-28 11:20:50
Message-ID: 460241cc-7b49-b793-fa50-c12898d4b332 () oracle ! com
[Download RAW message or body]


Hi, Thank you for reviewing the compiler changes.

On 7/28/20 3:31 AM, Christian Hagedorn wrote:
> Hi Coleen
>
> On 24.07.20 15:10, coleen.phillimore@oracle.com wrote:
>> incremental webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8250042.02.incr/webrev
>> full webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8250042.02/webrev
>
> Thanks for cleaning this up! The compiler changes look good to me.
>
> Just a minor comment (no new webrev required):
> - arm.ad:8873 & x86_32.ad:13321: There is an extra whitespace before ")"

Fixed!
Thanks,
Coleen
>
> Best regards,
> Christian
>
>> Thanks,
>> Coleen
>>
>>
>> On 7/24/20 8:23 AM, coleen.phillimore@oracle.com wrote:
>>>
>>> Thanks for looking at this.
>>>
>>> On 7/24/20 1:01 AM, David Holmes wrote:
>>>> Hi Coleen,
>>>>
>>>> On 24/07/2020 2:58 am, coleen.phillimore@oracle.com wrote:
>>>>> See bug for more details.  I've been running into these names a 
>>>>> lot lately.   Many of these names are in JVMTI.
>>>>>
>>>>> Tested with tier1 on all Oracle platforms and built on non-Oracle 
>>>>> platforms.
>>>>>
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2020/8250042.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8250042
>>>>
>>>> src/hotspot/cpu/*/*.ad
>>>>
>>>> These still refer to "method oop" and method_oop in a number of 
>>>> places.
>>>
>>> Yes, I only replaced method_oop in the shared code and not in the AD 
>>> code.  method_oop can be the name of a parameter and using "sed" to 
>>> change it to "method" doesn't work.   Somebody who understands this 
>>> code and looks at it will have to make the rest of the changes.
>>>
>>> What I did was replace "method oop" with "method" and "methodOop" 
>>> with "method" in all the sources.  I replaced "method_oop" with 
>>> "method" or "checked_method" in the shared sources.
>>>
>>>>
>>>> src/hotspot/share/adlc/adlparse.cpp
>>>>
>>>> +  frame->_interpreter_method_oop_reg = parse_one_arg("method reg 
>>>> entry");
>>>>
>>>> I guess I'm not understanding the scope of this renaming - why is 
>>>> _interpreter_method_oop_reg not renamed as well? Should this (and 
>>>> other uses) be parsed as method-(oop-reg) rather than 
>>>> (method-oop)-reg?
>>>
>>> I don't know this code, so I'd rather not change any more of it. The 
>>> comment makes sense changed, even though the variable name still 
>>> refers to method_oop.
>>>
>>> Thanks,
>>> Coleen
>>>>
>>>> Otherwise all okay.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> Coleen
>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]