Request for reviews(M): 7193318: C2: remove number of inputs requirement from Node's new operator

Christian Thalinger christian.thalinger at oracle.com
Mon Sep 24 18:12:49 PDT 2012


On Sep 24, 2012, at 12:04 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:

> 
> On Sep 24, 2012, at 11:57 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> 
>> On Sep 24, 2012, at 11:09 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 
>>> I regenerated webrev using Tom's webrev script so that udiffs are more clear:
>>> 
>>> http://cr.openjdk.java.net/~kvn/7193318/webrev/
>> 
>> Looks great.  Thanks for doing this cleanup!
> 
> One minor question/suggestion:  do we want to replace phase->C and friends with a local C variable?
> 
> -         mul_hi = phase->transform(new (phase->C) ConvL2INode(mul_hi));
> +         mul_hi = phase->transform(new (C) ConvL2INode(mul_hi));
> 
> Would make the code more compact and more readable.

...but it would be a too big change to integrate it into this one.  Let's file another RFE for that.

-- Chris

> 
> -- Chris
> 
>> 
>> -- Chris
>> 
>>> 
>>> Typo "necassary":
>>> 
>>> +   // Allocate memory for the necassary number of edges.
>>> 
>>> We do such alignment for sparc only. See comment in allocation.hpp. Please, change question to statement:
>>> 
>>> +     // Do we really need to allocate space for _in array to have
>>> +     // double alignment? May be we do since we are casting it to be
>>> +     // Node**.
>>> 
>>> Otherwise looks good.
>>> 
>>> thanks,
>>> Vladimir
>>> 
>>> Bharadwaj Yadavalli wrote:
>>>> De-couple memory allocation for edges from memory allocation for Node object.
>>>> This allows use of placement new operator of Node, viz., new(size_t, Compile *)
>>>> instead of node(size_t, Compile *, int) thereby eliminating the need to specify
>>>> the number of edges in the new operator.
>>>> Deleted placement new operator of Node - node(size_t, Compile *, int).
>>>> Testing done: jtreg, and JPRT
>>>> Benchmarking: refworkload.
>>>> Thanks,
>>>> Bharadwaj
>> 
> 



More information about the hotspot-compiler-dev mailing list