RFR: 8257146: C2: extend the scope of StressGCM

[Vladimir Kozlov]

On Mon, 30 Nov 2020 08:57:39 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> From the archives, the original discussion about StressGCM:
>>  https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2013-February/009777.html 
>> 
>> It was explicitly putting the loads in lower frequency block only, because as Vladimir says:
>>  > There is an other old bug which you may hit if you randomize placement
>>  > in gcm: 6831314. I still did not find a solution which does not
>>  > introduce performance regression. What saves us is placing loads into
>>  > "cheaper" low frequency block (which is most nested block where load's
>>  > result is used).
>> 
>> Is that not a problem anymore?
>
>> From the archives, the original discussion about StressGCM:
>> https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2013-February/009777.html
>> 
>> It was explicitly putting the loads in lower frequency block only, because as Vladimir says:
>> 
>> > There is an other old bug which you may hit if you randomize placement
>> > in gcm: 6831314. I still did not find a solution which does not
>> > introduce performance regression. What saves us is placing loads into
>> > "cheaper" low frequency block (which is most nested block where load's
>> > result is used).
>> 
>> Is that not a problem anymore?
> 
> Thanks for the pointer, Aleksey!
> 
> The [JIRA bug report for 6831314](https://bugs.openjdk.java.net/browse/JDK-6831314?focusedCommentId=14128897) was closed in 2017 with Vladimir's comment _"Will not fix. As evaluation said it is only observed with modified C2 code"_, so this particular case does not seem to be a problem anymore. In any case, my reasoning is that it is good to know if there are other cases where C2 relies on GCM heuristics for correctness, regardless of what we do about it.
> 
> @vnkozlov, what do you think?

First, StressGCM is not used in production only for testing - it should not affect released JDK.
Second,  I think  it would be nice to try reproduce 6831314 issue again with latest JDK. We did several changes with nodes types casting which may fixed it. And it should not prevent us from widening the scope of testing with StressGCM to find other issues now.
When @shipilev implemented this flag first, my concern was that we would have to spend a lot of resources to clean up found issues. Now we have more engineers and we should do the clean up.

@robcasloz  I suggest additionally to run hs-precheckin-comp with StressGCM ON.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1469