RFR(L) 8034198: Cleanup and re-factorize PhaseChaitin::build_ifg_physical()
niclas.adlertz at oracle.com
Wed Jan 15 05:45:10 PST 2014
I found a commented section that I forgot to remove in chaitin.hpp:
// stores the first low-to-high transition (starting from the top of block)
Updated the webrev:
On 2014-01-15 14:26, Niclas Adlertz wrote:
> Hi Rickard,
> Thank you for your comments.
> Kind Regards,
> Niclas Adlertz
> On 2014-01-14 15:03, Rickard Bäckman wrote:
>> Just a few comments on the C++ parts. I haven't verified that the logic
>> is still the same.
>> 1) It seems like there are a couple of functions that could be created
>> in the new Pressure class to avoid code duplication. One example is the
>> check_for_high_pressure_block method.
>> 2) All places that checks lrg._is_float also check lrg._is_vector. That
>> check could be made into a boolean method on the LRG instead. The same
>> for is_UP() & mask_size().
>> 3) To make code more generic the int_pressure and float_pressure
>> instances could be passed the INTPRESSURE and FLOATPRESSURE and keep
>> those as instance variables.
>> Good work.
>> On 01/14, Niclas Adlertz wrote:
>>> Hi all,
>>> This is a first step to clean up the register allocator in C2. In this change we have divided the very long method build_ifg_physical() into many smaller ones, making it easier to see the steps when building the IFG (and computing the block pressure).
>>> We have also cleaned up old comments and improved old code, both by better naming and by simplifying expressions.
>>> I personally think that the easiest way to review this change is to have the old and new ifg.cpp side by side, and start from build_ifg_physical().
>>> The next step is to create a new class for these methods, isolating them and removing them from PhaseChaitin.
>>> WEBREV: http://cr.openjdk.java.net/~adlertz/JDK-8031498/webrev00/
>>> BUG: https://bugs.openjdk.java.net/browse/JDK-8031498
>>> Kind Regards,
>>> Niclas Adlertz
More information about the hotspot-compiler-dev