RFR: 8370077: C2: make Compile::_major_progress a boolean
Marc Chevalier
mchevalier at openjdk.org
Wed Oct 22 07:39:20 UTC 2025
On Tue, 21 Oct 2025 17:32:08 GMT, Vladimir Kozlov <kvn 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
>
> Or you can keep temporary (just for testing this PR and remove it before integration) original logic in debug VM to compare result of `major_progress()`.
@vnkozlov I fear I don't understand what you're suggesting.
I've tried to add in my `set_major_progress(bool)` an assert to check we are not in the 3rd case, the one where the assignment-semantics and the OR-semantics mismatch (that is with `progress` parameter (old progress) unset and current `_major_progress` set). And indeed the assert does not fire in tier1-6+some other internal testing.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/27912#issuecomment-3430850080
More information about the hotspot-compiler-dev
mailing list