RFR: 8370077: C2: make Compile::_major_progress a boolean
Emanuel Peter
epeter at openjdk.org
Tue Oct 21 16:39:17 UTC 2025
On Tue, 21 Oct 2025 08:07:15 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:
> Simply change `Compile::_major_progress` from `int` to `bool` since we are only checking if it's non-zero.
>
> There is one detail, we used to have
>
> void restore_major_progress(int progress) { _major_progress += progress; }
>
>
> It is used after some verification code (maybe not only?) that may reset the major progress, using the progress saved before the said code.
>
> It has a weird semantics:
>
> Progress before | Progress after verification | Progress after restore
> ----------------|-----------------------------|-----------------------
> 0 | 0 | 0
> 1 | 0 | 1
> 0 | 1 | 1
> 1 | 1 | 2
>
> It is rather a or than a restore, and a proper boolean version of that would be
>
> void restore_major_progress(bool progress) { _major_progress = _major_progress || progress; }
>
> but then, I'd argue the name is confusing. It also doesn't fit so well the idea that we just want to be back to the situation before the verification code. I suspect the unsaid assumption, is that the 3rd line (progress clear before, set by verification) is not possible. Anyway, I've tried with this or-semantics, or with a more natural
>
> void set_major_progress(bool progress) { _major_progress = progress; }
>
> that actually restore what we saved. Both pass (tier1-6 + some internal tests). Thus, I prefered the simpler semantics.
>
> Thanks,
> Marc
Thanks for working on this. I agree that the old `restore_major_progress` semantics was a little strange, and hard to understand. So good you are trying to simplify.
I know that all your testing passed... but if there was a bug we may not notice purely with testing. There could also be performance regressions, in cases where we then don't continue optimizing because we messed up and set `major_progress` where it should not be set. Do you think it could make sense to run some performance testing?
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27912#issuecomment-3427587076
More information about the hotspot-compiler-dev
mailing list