RFR(L) 8001532: C2 node files refactoring

Morris Meyer morris.meyer at oracle.com
Mon Mar 31 20:06:50 UTC 2014


Sorry - used the XCode setting for Java code.  Here's the changed webrev.

WEBREV - http://cr.openjdk.java.net/~morris/8001532.05

--mm

On 3/31/14, 2:02 PM, Vladimir Kozlov wrote:
> Hi Morris,
>
> Everything in place but new files have wrong indention. We use 2 bytes 
> stepping, in your new files it is 4. Please, fix.
>
> thanks,
> Vladimir
>
> On 3/31/14 9:53 AM, Morris Meyer wrote:
>> Vladimir,
>>
>> Thanks for your review.  Here is the webrev modified from your 
>> feedback.  This has been through JPRT.
>>
>>          --mm
>>
>>
>> WEBREV - http://cr.openjdk.java.net/~morris/8001532.04
>>
>>
>> On 3/18/14, 6:11 PM, Vladimir Kozlov wrote:
>>> The big comment in connode.cpp belongs to cmove nodes and should be 
>>> moved into movenode.cpp.
>>>
>>> Why you kept threadnode.hpp file?
>>>
>>> Otherwise it looks good. I would ask to not push it now because it 
>>> is interfering with my RTM changes (I added
>>> Opaque3Node). May be next week.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 3/18/14 1:34 PM, Morris Meyer wrote:
>>>> Thanks for the review Vladimir.
>>>>
>>>> Here is the webrev modified from yours and Christian's feedback.
>>>>
>>>>          --mm
>>>>
>>>> JPRT - hotspotest - 2014-03-18-190819.mameyer.8001532
>>>> WEBREV - http://cr.openjdk.java.net/~morris/8001532.03
>>>>
>>>> On 3/17/14, 6:20 PM, Vladimir Kozlov wrote:
>>>>> File names usually match base class name of ideal nodes. Please 
>>>>> change:
>>>>>
>>>>> constnode back to connode
>>>>> bitsnode   --> countbitsnode
>>>>> narrownode --> narrowptrnode
>>>>> optonode   --> opaquenode
>>>>>
>>>>> PartialSubtypeCheckNode class should be in new intrinsicnode file
>>>>> together with other similar classes from memnode files:
>>>>> StrIntrinsicNode and related, EncodeISOArrayNode.
>>>>>
>>>>> ThreadLocalNode can be kept in connode because it is kind of a
>>>>> constant pointer value.
>>>>>
>>>>> Put BinaryNode into movenode.hpp since it references cmove nodes.
>>>>>
>>>>> constnode.hpp is included into callnode.hpp so you don't need to
>>>>> include it into files which have callnode.hpp included. Yes, we 
>>>>> had it
>>>>> before but you are cleaning the code.
>>>>>
>>>>> thanks,
>>>>> Vladimir
>>>>>
>>>>> On 3/17/14 6:51 AM, Morris Meyer wrote:
>>>>>> Folks,
>>>>>>
>>>>>> Could I get a review for 8001532 - refactoring the old connode 
>>>>>> file in
>>>>>> the C2 source tree?
>>>>>>
>>>>>> I've broken connode into bitsnode, castnode, constnode, convertnode,
>>>>>> movenode, narrownode, optonode and threadnode.
>>>>>>
>>>>>> This change has been through JPRT.
>>>>>>
>>>>>>          --morris meyer
>>>>>>
>>>>>> JBS - https://bugs.openjdk.java.net/browse/JDK-8001532
>>>>>> WEBREV - http://cr.openjdk.java.net/~morris/8001532.01
>>>>
>>



More information about the hotspot-compiler-dev mailing list