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