[lworld] RFR: 8371357: [lworld] Remove EnableValhalla

Coleen Phillimore coleenp at openjdk.org
Fri Dec 12 16:21:40 UTC 2025


On Tue, 25 Nov 2025 13:37:46 GMT, Paul Hübner <phubner at openjdk.org> 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.

Except for EnableValhalla legacy comments that I think should be removed, this looks reasonable on it's own.  Does it pass our existing tier1-4 testing without updating problem lists ?

src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp line 104:

> 102:   assert_different_registers(obj, klass, len);
> 103: 
> 104:   // EnableValhalla legacy

I don't think this comment is necessary and kind of noisy everywhere, especially since there's Arguments::enable_preview() also tested.  The weird thing is that EnableValhalla is true but Arguments::enable_preview() is not always true so I see why it's good to have testing with both settings, we were exercising valhalla code even with enable-preview off.

src/hotspot/share/cds/cdsConfig.hpp line 211:

> 209: 
> 210:   // EnablePreview legacy
> 211:   static bool is_valhalla_preview() { return Arguments::enable_preview(); }

You might want to change all is_valhalla_preview() in CDS to just Arguments::enable_preview().  Or not.  Not completely necessary I guess.

test/hotspot/jtreg/serviceability/jvmti/valhalla/HeapDump/HeapDump.java line 153:

> 151: 
> 152:             // -XX:+PrintInlineLayout is debug-only arg
> 153:             LingeredApp.startApp(theApp, "--enable-preview", "-XX:+UnlockDiagnosticVMOptions",

Is there an if (debugvm) test that you need here?

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

PR Review: https://git.openjdk.org/valhalla/pull/1759#pullrequestreview-3568470576
PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2611403488
PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2611412940
PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2611429239


More information about the valhalla-dev mailing list