RFR(L) 8034198: Cleanup and re-factorize PhaseChaitin::build_ifg_physical()

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Jan 16 00:11:09 PST 2014


Changes are good. I have few notes.

Vectors are separate from floats. Please, rename the method to is_float_or_vector().

class Pressure. Field names start with '_' to separate then from variables.

Why you don't like to path pointer to liveout? It helped only in few places but code in interfere_with_live() become 
complicated (IndexSet& liveout and then elements(&liveout)).


On 1/15/14 5:45 AM, Niclas Adlertz wrote:
> Hi again,
> 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)
> //uint final_high_pressure_index;
> Updated the webrev:
> http://cr.openjdk.java.net/~adlertz/JDK-8031498/webrev02/
> On 2014-01-15 14:26, Niclas Adlertz wrote:
>> Hi Rickard,
>> Thank you for your comments.
>> http://cr.openjdk.java.net/~adlertz/JDK-8031498/webrev01/
>> 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.
>>> /R
>>> 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 mailing list