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

Paul Hübner phubner at openjdk.org
Tue Dec 16 10:19:30 UTC 2025


On Mon, 15 Dec 2025 22:30:27 GMT, Daniel D. Daugherty <dcubed at openjdk.org> 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


More information about the valhalla-dev mailing list