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 12:04:48 PDT 2012
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.
-- 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