RFR: 8356647: C2: Excessively strict assert in PhaseIdealLoop::do_unroll
Marc Chevalier
mchevalier at openjdk.org
Thu May 22 15:39:51 UTC 2025
On Wed, 21 May 2025 14:48:16 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> This assert seems a bit too tight. See the JBS issue to check the math: the bound of `trip_count` should be `<= 2^31`, while the current bound is ` < (julong)max_juint/2` = floor((2^32-1)/2) = (2^32-2) / 2 = 2^31-1.
>
> Drive-by comment: Were you able to extract a regression test that does not require the stress peeling flag?
Thanks to @chhagedorn's help, there comes a test without stress flag. Thanks! We also looked more in details, and we cannot reach the case where new trip count would be 2^31 because this branch is taken only if `has_exact_trip_count()` is true, which means the trip count was set with `set_exact_trip_count` which happens only here:
https://github.com/openjdk/jdk/blob/e961b13cd68bc352b86af17c7e53df8537519beb/src/hotspot/share/opto/loopTransform.cpp#L133-L141
so only with a trip count < `max_juint` = 2^32-1, so at most 2^32-2. We are safe! Overall, the patch is now:
- an additional assert about the old trip count
- changing `<` into `<=` making the sanity just as obvious
- adding a test
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25295#issuecomment-2901694311
More information about the hotspot-compiler-dev
mailing list