RFR: 8330157: C2: Add a stress flag for bailouts

[Thomas Stuefe]

On Wed, 12 Jun 2024 15:04:14 GMT, Daniel Skantz <duke at openjdk.org> wrote:

> > This looks useful.
> > Is it planned to also test bailouts for C1 with the same switch?
> 
> Thanks for the review, Thomas! I did not consider stressing C1 bailouts. If this method is shown to be effective for C1 too, could we consider adding a separate flag for that at a later time?

A later time is fine. I would probably aim to reuse the flag though.

>> src/hotspot/share/opto/compile.hpp line 847:
>> 
>>> 845:     return false; // debug builds assert on bailouts.
>>> 846: #endif
>>> 847:     if (!_stress_seed || (random() % invprob)) {
>> 
>> - nit, unless comparing boolean expressions, we use explicit comparisons, not !
>> - but the _stress_seed != 0 exclusion is somewhat odd. We allow the seed to be 0 - since `-XX:StressSeed=0` is valid. So, either we should make sure `StressSeed` can never be 0 by restricting its valid range to >=1. Then, here (or better, inside Compile::random()) we should assert that seed != 0. Alternatively, if StressSeed=0 is supposed to be valid, this exclusion here can be removed. Note that it does not get mirrored by other uses of `Compile::random`.
>
> It was intended to be a workaround until JDK-8333956 can be pushed, to check if the stress seed has been initialized yet -- but it's a good point that StressSeed=0 is now excluded. We could use an additional variable to indicate that the stress seed has been initialized.

Yes, or just declare StressSeed=0 being invalid. I dimly remember running into problems with os::random and 0 seeds at some time. Cannot remember the issue, but since then, I try to avoid 0 seeds.

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

PR Comment: https://git.openjdk.org/jdk/pull/19646#issuecomment-2164378961
PR Review Comment: https://git.openjdk.org/jdk/pull/19646#discussion_r1637538765