RFR: 8330157: C2: Add a stress flag for bailouts
Daniel Skantz
duke at openjdk.org
Wed Jun 12 08:14:12 UTC 2024
On Tue, 11 Jun 2024 10:10:23 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> This patch adds a diagnostic/stress flag for C2 bailouts. It can be used to support testing of existing bailouts to prevent issues like [JDK-8318445](https://bugs.openjdk.org/browse/JDK-8318445), and can test for issues only seen at runtime such as [JDK-8326376](https://bugs.openjdk.org/browse/JDK-8326376). It can also be useful if we want to add more bailouts ([JDK-8318900](https://bugs.openjdk.org/browse/JDK-8318900)).
>>
>> We check two invariants.
>> a) Bailouts should be successful starting from any given `failing()` check.
>> b) The VM should not record a bailout when one is pending (in which case we have continued to optimize for too long).
>>
>> a), b) are checked by randomly starting a bailout at calls to `failing()` with a user-given probability.
>>
>> The added flag should not have any effect in debug mode.
>>
>> Testing:
>>
>> T1-5, with flag and without it. We want to check that this does not cause any test failures without the flag set, and no unexpected failures with it. Tests failing because of timeout or because an error is printed to output when compilation fails can be expected in some cases.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19646#discussion_r1636018055
More information about the hotspot-compiler-dev
mailing list