RFR(XS) 8032894: Remove dead code in Pressure::lower
Vladimir Kozlov
vladimir.kozlov at oracle.com
Wed Jan 29 14:31:20 PST 2014
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().
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
>
> 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
>
More information about the hotspot-compiler-dev
mailing list