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

Niclas Adlertz niclas.adlertz at oracle.com
Tue Feb 4 00:42:51 PST 2014

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 > finalpressure 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 currentpressure and finalpressure are computed in PhaseChaitin::computeinitialblockpressure(). We loop through all live ranges that are in the LIVEOUT of the block, and raise the currentpressure (for either int or float) for each live range. If the currentpressure is bigger than the finalpressure (which is initially 0), we set the finalpressure to be currentpressure. We check this each time we increase the currentpressure. This is done in Pressure::raise(). (I.e. the finalpressure is the highest value of currentpressure that we ever encounter in the block) It is not accurate, it could be higher. You overwrite it in checkforhighpressuretransitionatfatproj(). In a way it is accurate, because at a fatproj the currentpressure will be raised and then lowered right after (stepping backwards) since the fatproj is only "live" at its definition. (That's why we never change the currentpressure at a fatproj but still look for a finalpressure 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 highpressurelimit is passed into the constructor of the Pressure class. This is done in the beginning of the buildifgphysical(): Pressure intpressure(lastinst + 1, INTPRESSURE); Pressure floatpressure(lastinst + 1, FLOATPRESSURE); How we can get case (currentpressure == highpressurelimit)? 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 LIVEOUT when stepping backwards in the block). So if currentpressure is greater than highpressurelimit, and we lower the currentpressure, we might hit the case when currentpressure == highpressurelimit. 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 finalpressure when raising? Kind Regards, Niclas Adlertz Kind Regards, Niclas Adlertz

On 2014-01-29 19:53, Vladimir Kozlov wrote: What is initial state of currentpressure, highpressurelimit, finalpressure? How we can get case (currentpressure == highpressurelimit)? May be we should replace the code with assert: assert(currentpressure <= finalpressure 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 LIVEOUT, (and lower the current pressure) and add input to the LIVEOUT (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 finalpressure. However the finalpressure can never increase when removing definitions (i.e. lower the current pressure) so the check for a new finalpressure 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