RFR: 8370077: C2: make Compile::_major_progress a boolean

Marc Chevalier mchevalier at openjdk.org
Tue Oct 21 12:01:43 UTC 2025


On Tue, 21 Oct 2025 11:51:29 GMT, Francesco Andreuzzi <fandreuzzi 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
>
> src/hotspot/share/opto/compile.hpp line 325:
> 
>> 323:   bool                  _allow_macro_nodes;     // True if we allow creation of macro nodes.
>> 324: 
>> 325:   bool                  _major_progress;        // Count of something big happening
> 
> Perhaps the comment should be updated too?

That makes sense. A suggestion? Maybe "Whether something big happened"?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27912#discussion_r2447889101


More information about the hotspot-compiler-dev mailing list