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

[Vladimir Kozlov]

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
>