RFR(XS) 8032894: Remove dead code in Pressure::lower

[Niclas Adlertz]

Thank you Vladimir.

Kind Regards,
Niclas Adlertz

On 2014-02-04 06:32, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 2/3/14 2:14 PM, Niclas Adlertz wrote:
>> Hi Vladimir,
>>
>> Here is an attempt at making it clearer by making all _*_pressure 
>> fields private.
>>
>> http://cr.openjdk.java.net/~adlertz/JDK-8032894/webrev01/
>>
>> Kind Regards,
>> Niclas Adlertz
>>
>> On 2014-01-30 00:22, Vladimir Kozlov wrote:
>>>  > Do you still want me to add the assert even though we only find a 
>>> new
>>>  > _final_pressure when raising?
>>>
>>> No, but I want you to look on my suggestion:
>>>
>>>  > May be all _*_pressure fields should be private and code which 
>>> updates
>>>  > them is located in one place. Then it would be more obvious that 
>>> code
>>>  > you are removing is useless.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 1/29/14 3:05 PM, Niclas Adlertz wrote:
>>>> Hi Vladimir,
>>>>
>>>> On 2014-01-29 23:31, Vladimir Kozlov wrote:> Hi Niclas
>>>>>
>>>>> On 1/29/14 2:02 PM, Niclas Adlertz wrote:
>>>>>> Hi Vladimir,
>>>>>>
>>>>>> The initial states of _current_pressure and _final_pressure are 
>>>>>> computed
>>>>>> in PhaseChaitin::compute_initial_block_pressure().
>>>>>> We loop through all live ranges that are in the LIVE_OUT of the 
>>>>>> block,
>>>>>> and raise the _current_pressure (for either int or float) for 
>>>>>> each live
>>>>>> range. If the _current_pressure is bigger than the _final_pressure
>>>>>> (which is initially 0), we set the _final_pressure to be
>>>>>> _current_pressure. We check this each time we increase the
>>>>>> _current_pressure. This is done in Pressure::raise(). (I.e. the
>>>>>> _final_pressure is the highest value of _current_pressure that we 
>>>>>> ever
>>>>>> encounter in the block)
>>>>>
>>>>> It is not accurate, it could be higher. You overwrite it in
>>>>> check_for_high_pressure_transition_at_fatproj().
>>>> In a way it is accurate, because at a fat_proj the _current_pressure
>>>> will be raised and then lowered right after (stepping backwards) since
>>>> the fat_proj is only "live" at its definition.
>>>> (That's why we never change the _current_pressure at a fat_proj but
>>>> still look for a _final_pressure change).
>>>>
>>>>>
>>>>> May be all _*_pressure fields should be private and code which 
>>>>> updates
>>>>> them is located in one place. Then it would be more obvious that code
>>>>> you are removing is useless.
>>>>>
>>>>>>
>>>>>> The _high_pressure_limit is passed into the constructor of the 
>>>>>> Pressure
>>>>>> class. This is done in the beginning of the build_ifg_physical():
>>>>>>
>>>>>> Pressure int_pressure(last_inst + 1, INTPRESSURE);
>>>>>> Pressure float_pressure(last_inst + 1, FLOATPRESSURE);
>>>>>>
>>>>>>> How we can get case (_current_pressure == _high_pressure_limit)?
>>>>>> When we lower the pressure, it is assumed that we lower each time 
>>>>>> by 1
>>>>>> (this happens every time we remove a definition from the LIVE_OUT 
>>>>>> when
>>>>>> stepping backwards in the block). So if _current_pressure is greater
>>>>>> than _high_pressure_limit, and we lower the _current_pressure, we 
>>>>>> might
>>>>>> hit the case when _current_pressure == _high_pressure_limit. If 
>>>>>> so, we
>>>>>> have found a Low to High pressure transition in the block. (Low 
>>>>>> to High
>>>>>> when starting from the top of the block)
>>>>>> However, there is one problem with this case, since the pressure 
>>>>>> could
>>>>>> be lowered by more than 1. (Long or Double will lower it by 2). 
>>>>>> So we
>>>>>> might miss a transition when lowering. There is already a bug
>>>>>> (JDK-8032886) filed for this which will be out soon. I didn't fix 
>>>>>> this
>>>>>> bug in the clean up since I wanted them to be separate commits.
>>>>>
>>>>> Okay, sounds good.
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>
>>>> Do you still want me to add the assert even though we only find a new
>>>> _final_pressure when raising?
>>>>
>>>> Kind Regards,
>>>> Niclas Adlertz
>>>>
>>>>>>
>>>>>> Kind Regards,
>>>>>> Niclas Adlertz
>>>>>>
>>>>>>
>>>>>> On 2014-01-29 19:53, Vladimir Kozlov wrote:
>>>>>>> What is initial state of _current_pressure, _high_pressure_limit,
>>>>>>> _final_pressure? How we can get case (_current_pressure ==
>>>>>>> _high_pressure_limit)? May be we should replace the code with 
>>>>>>> assert:
>>>>>>>
>>>>>>> assert(_current_pressure <= _final_pressure
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 1/29/14 4:09 AM, Niclas Adlertz wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> When building the physical IFG, we step backwards in each 
>>>>>>>> block, and
>>>>>>>> remove things that are defined from the LIVE_OUT, (and lower the
>>>>>>>> current
>>>>>>>> pressure) and add input to the LIVE_OUT (and raising the current
>>>>>>>> pressure). Each time we lower or raise the current pressure, we
>>>>>>>> check if
>>>>>>>> it's bigger than the current maximum pressure, known as
>>>>>>>> final_pressure.
>>>>>>>> However the final_pressure can never increase when removing
>>>>>>>> definitions
>>>>>>>> (i.e. lower the current pressure) so the check for a new
>>>>>>>> final_pressure
>>>>>>>> in Pressure::lower is useless.
>>>>>>>>
>>>>>>>> webrev: http://cr.openjdk.java.net/~adlertz/JDK-8032894/webrev00/
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8032894
>>>>>>>>
>>>>>>>> Kind Regards,
>>>>>>>> Niclas Adlertz
>>