[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