RFR(S): 8220374: C2: LoopStripMining doesn't strip as expected

[Doerr, Martin]

Hi Roland,

> Oh wow. We should probably mark this for a backport then, right?
Makes sense. I suggest to backport JDK-8219584, too. It's needed for the test to work correctly.

> Have you tested with a release build? Because SafepointALot is a debug flag and will probably fail.
It has been changed to develop by the change mentioned above.

The local variable "outer_cmp" is unused and may possibly cause build warnings.
Besides that, the fix looks good to me.

Thanks for fixing it so quickly.

Best regards,
Martin


-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Tobias Hartmann
Sent: Mittwoch, 13. März 2019 15:10
To: Roland Westrelin <rwestrel at redhat.com>; hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): 8220374: C2: LoopStripMining doesn't strip as expected

Hi Roland,

On 13.03.19 10:04, Roland Westrelin wrote:
> http://cr.openjdk.java.net/~roland/8220374/webrev.00/
> 
> Fix for 8193597 accidentally broke loop strip mining: that change caused
> the inner and outer loop to have the same exit condition. So loop strip
> mining has been effectively disabled for more than a year (a month after
> it was pushed !?).

Oh wow. We should probably mark this for a backport then, right?

> The fix above correctly sets the inner and outer loop exit
> condition. Initially, when the loop is strip mined, the inner loop exit
> condition is left as is and the outer loop exit condition is set to
> always exit. After optimizations, the outer loop exit condition must be
> set to be the same as the inner loop exit condition and then, the inner
> loop exit condition must be adjusted so the inner loop runs for no more
> than LoopStripMiningIter iterations.

Looks reasonable to me.

> Thanks to Martin Doerr for the test case and the bug report.

Have you tested with a release build? Because SafepointALot is a debug flag and will probably fail.

Also, please add some newlines to the @run statement.

Thanks,
Tobias