On Tue, 25 Nov 2025 13:37:46 GMT, Paul Hübner <[email protected]> wrote:
> Hi all,
>
> This removes the `EnableValhalla` in favour of the `--enable-preview` flag.
> Concretely:
> * I've replaced most of the `EnableValhalla` checks with
> `Arguments::is_valhalla_enabled()`.
> * Some checks were redundant and could be removed entirely.
> * I've made the `EnableValhalla` flag obsolete.
> * Some tests had to be updated.
>
> This greatly changes the semantics of tests. I've refined some test groups to
> make it easier.
>
> Testing: tiers 1-4.
Not a full review, but I think this looks good. If this is accepted you'll have
to add includes of arguments.hpp to all places where you have added a call to
`is_valhalla_enabled()`.
src/hotspot/share/runtime/arguments.cpp line 4005:
> 4003: FLAG_SET_DEFAULT(BytecodeVerificationRemote, true);
> 4004: }
> 4005: if (!Arguments::is_valhalla_enabled() || (is_interpreter_only() &&
> !CDSConfig::is_dumping_archive() && !UseSharedSpaces)) {
Can this be just:
Suggestion:
if (!is_valhalla_enabled() || (is_interpreter_only() &&
!CDSConfig::is_dumping_archive() && !UseSharedSpaces)) {
-------------
PR Review:
https://git.openjdk.org/valhalla/pull/1759#pullrequestreview-3572302794
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1759#discussion_r2614606046
