On Mon, 15 Dec 2025 22:30:27 GMT, Daniel D. Daugherty <[email protected]> wrote:
>> Paul Hübner has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 23 commits: >> >> - Use the new function for the Parallel changes. >> - Merge branch 'lworld' into JDK-8371357 >> - Remove wrong comment. >> - Fix imports. >> - Reviewer comments. >> - Copyright. >> - Revert "Don't synchronize on Double." >> >> This reverts commit cbf849c2ce6059af2b43b3dd8cb42445eab4df2b. >> - Accidentally changed too many enable_preview. >> - Make heap dump test require debug VM. >> - Change to arguments. >> - ... and 13 more: >> https://git.openjdk.org/valhalla/compare/8c0c190b...58fb031a > > src/hotspot/share/oops/arrayOop.hpp line 59: > >> 57: // Given a type, return true if elements of that type must be aligned >> to 64-bit. >> 58: static bool element_type_should_be_aligned(BasicType type) { >> 59: if (type == T_FLAT_ELEMENT) { > > You don't need `EnableValhalla` or `Arguments::is_valhalla_enabled()` as long > as all callers > of `element_type_should_be_aligned()` only pass known valid BasicType values. > If any caller > passes a non-valid BasicType value as part of a "probing call", then a call > with a value that > happens to match `T_FLAT_ELEMENT` by a VM with > `Arguments::is_valhalla_enabled() == false` > will return `true` unexpectedly. I think this should be fine because `T_FLAT_ELEMENT` is hardcoded to `15` regardless of if Valhalla/preview is enabled. > src/hotspot/share/runtime/arguments.cpp line 4006: > >> 4004: FLAG_SET_DEFAULT(BytecodeVerificationRemote, true); >> 4005: } >> 4006: if (is_valhalla_enabled() || (is_interpreter_only() && >> !CDSConfig::is_dumping_archive() && !UseSharedSpaces)) { > > Hmmm... The original logic is `!EnableValhalla` so shouldn't this be: > `!is_valhalla_enabled()`? You're right. Good spot! ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2622631986 PR Review Comment: https://git.openjdk.org/valhalla/pull/1759#discussion_r2622645808
