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

[Niclas Adlertz]

Vladimir,

Thank you for your comments!

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

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

> 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)).
I thought it looked better to send in the liveout as a reference since 
it was allocated on stack in build_ifg_physical:
IndexSet liveout(_live->live(block));
But if you want me revert the change I can do that.


http://cr.openjdk.java.net/~adlertz/JDK-8031498/webrev03/

Kind Regards,
Niclas Adlertz


On 2014-01-16 09:11, Vladimir Kozlov wrote:
> Niclas,
>
> 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)).
>
> Thanks,
> Vladimir
>
> 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
>>>
>>