RFR: 8319813: Remove upper limit on number of compiler phases in phasetype.hpp [v3]
Emanuel Peter
epeter at openjdk.org
Tue Nov 21 07:55:12 UTC 2023
On Mon, 20 Nov 2023 14:47:43 GMT, Daniel Lundén <duke at openjdk.org> wrote:
>> This changeset removes the implicit upper limit on the number of compiler phases in `phasetype.hpp`. The limit was due to the 64-bit mask used in `Compile::should_print_phase`, causing the `assert(cpt < 64, "out of bounds");` in `phasetype.hpp` to trigger when increasing the number of phases to more than 64.
>>
>> Changes:
>> - Replace the 64-bit mask with a bit map (`utilities/bitMap.hpp`).
>> - Clean up the `PhaseNameValidator` interface by allocating the mask internally.
>> - Move the check for whether a mask is non-zero ("is set") into `PhaseNameValidator`.
>> - Add a method `should_print_phase` to `DirectiveSet`, simplifying the `if`-condition in `Compile::should_print_phase`.
>>
>> ### Testing
>> Platforms: windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64.
>> - `tier1`
>> - HotSpot parts of `tier2` and `tier3`
>
> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>
> Inline single-use function for empty check
Looks much better already, just a few more comments :)
src/hotspot/share/compiler/compilerDirectives.cpp line 300:
> 298: }
> 299:
> 300: DirectiveSet::DirectiveSet(CompilerDirectives* d) :_inlinematchers(nullptr), _directive(d), _ideal_phase_name_set(PHASE_NUM_TYPES, mtCompiler) {
Needs a space after the colon `:`. But even better would be to put the initializations on a new line each, with 2 spaces indentation, and the opening bracket on a new line by itself.
src/hotspot/share/compiler/compilerDirectives.cpp line 622:
> 620:
> 621: set->_intrinsic_control_words = src->_intrinsic_control_words;
> 622: set->_ideal_phase_name_set.set_from(src->_ideal_phase_name_set);
The advantage of passing the set/mask directly is that here you could also use your `set_ideal_phase_name_set` method.
-------------
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16729#pullrequestreview-1741300862
PR Review Comment: https://git.openjdk.org/jdk/pull/16729#discussion_r1400145823
PR Review Comment: https://git.openjdk.org/jdk/pull/16729#discussion_r1400149122
More information about the hotspot-compiler-dev
mailing list