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

Paul Hübner phubner at openjdk.org
Fri Dec 12 16:21:44 UTC 2025


On Thu, 11 Dec 2025 17:13:03 GMT, Coleen Phillimore <coleenp 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.
>
> 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.

Will remove.

> 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?

Good catch.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2614549751
PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2614558311


More information about the valhalla-dev mailing list