RFR: 8370077: C2: make Compile::_major_progress a boolean [v2]

[Marc Chevalier]

On Wed, 22 Oct 2025 07:46:35 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 | What would be the assignment semantics
>> ----------------|-----------------------------|-----------------------|-
>> 0               | 0                           | 0 | 0
>> 1               | 0                           | 1 | 1
>> 0               | 1                           | 1 | 0 (mismatch!)
>> 1               | 1                           | 2 | 1 (same truthiness)
>> 
>> 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
>
> Marc Chevalier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adapt the comment

There are 2 things:
1. `restore_major_progress` from addition (or OR) semantics to assignment (@dean-long's concern): I've added

void          set_major_progress(bool progress) { precond(!(!progress && _major_progress)); _major_progress = progress; }

To see if we are ever in the case that the `old_progress` is false and `_major_progress` is true. That is the only case where the former OR-semantics is not the same as the new set semantics. It passes tier1-6 + other internal tests. I can replace the assignment with a `||` (the boolean `+`) if we still have doubts, but then, it seems tests are not exercising this path.
2. Is the type change correct overall. I've did something as @vnkozlov describes: have side by side the bool and the int version of the major progress, have the methods acts on both at the same time: on the int as it used to, on the bool as I propose here. Add the proposed assert in the getter. I've also made sure to assign both the int and the bool version for the 2 places in `compile.cpp` that assign `_major_progress` directly. It passes tier1-3 + other internal tests. This also makes sure there is no observable difference between the `+=` for the `int` version, and the assignment for the `bool` version.

Here is what I've tested with:
[testing.patch](https://github.com/user-attachments/files/23091637/testing.patch)

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

PR Comment: https://git.openjdk.org/jdk/pull/27912#issuecomment-3435421146