On Fri, 4 Sep 2026 04:28:54 GMT, Stefan Karlsson <[email protected]> wrote:

>> JEP 401 delivered the preview feature of Value Objects.
>> 
>> During the lifetime of the Valhalla project there were many ideas and names, 
>> and at one point the value types were called inline types. The HotSpot code 
>> was changed to use the name "inline types" instead of "value types" (and 
>> inline klass instead of value klass) 
>> [JDK-8249257](https://bugs.openjdk.org/browse/JDK-8249257). 
>> 
>> Later the concept and name "inline type" were dropped, but HotSpot retained 
>> the name. This causes confusion when working with the code. Some reasons:
>> 
>> 1) There's no obvious connection to what's described in JEP 401 or the Java 
>> code.
>> 
>> 2) It is easy to incorrectly assume that these are types that have been 
>> flattened (inlined) into other objects.
>> 
>> 3) The name clash with C++'s concept of inlining
>> 
>> My proposal is that we revert back to talking about "values" instead of 
>> "inlines", and that we do this before JDK 28 ships, so that we don't have a 
>> release where we use the "inline" terminology.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Stefan Karlsson has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge remote-tracking branch 'upstream/master' into 
> valhalla_rename_inline_to_value
>  - Rename InlineLayoutInfo
>  - Merge remote-tracking branch 'upstream/master' into 
> valhalla_rename_inline_to_value
>  - inline_alloc -> value_type_alloc
>  - Rename PrintInlineLayout
>  - Rename inline_args functions
>  - Fix alignments
>  - Fix inline entry point names
>  - Small fixes
>  - 8391728: Rename inline klass / type to value klass / type to match Java 
> side of JEP 401

Automatic audit finds these spots:

src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp line 489:

> 487:       __ test_oop_is_not_value_type(r0, rscratch2, skip, /* can_be_null= 
> */ false);
> 488: 
> 489:       // Load fields from a buffered value with an value class specific 
> handler

Nit: `a value`

src/hotspot/share/cds/aotMapLogger.cpp line 893:

> 891:             _st->print("Flat value type field '%s':", 
> vk->name()->as_C_string());
> 892:           } else {
> 893:             _st->print("Flat inline null-free type field '%s':", 
> vk->name()->as_C_string());

Missed the spot here, "inline". It would fail in 
`test/hotspot/jtreg/runtime/cds/AOTMapReader.java` test, AFAICS, which expects 
the new patterns?

src/hotspot/share/classfile/systemDictionary.cpp line 1122:

> 1120:   }
> 1121: 
> 1122:   InstanceKlass* k = ik->get_value_type_field_klass(field_index);

`_or_null` is dropped here. Semantic change?

src/hotspot/share/classfile/systemDictionary.cpp line 1164:

> 1162:                            "Cause: field type in LoadableDescriptors 
> attribute",
> 1163:                            name->as_C_string(), 
> ik->name()->as_C_string());
> 1164:   InstanceKlass* k = ik->get_value_type_field_klass(field_index);

`_or_null` is dropped here. Semantic change?

src/hotspot/share/runtime/fieldDescriptor.cpp line 211:

> 209:           st->cr();
> 210:         } else {
> 211:           st->print_cr("Flat inline null-free type field '%s':", 
> vk->name()->as_C_string());

Missed the spot here: "inline". It would fail in 
`test/hotspot/jtreg/runtime/cds/AOTMapReader.java` test, AFAICS, which expects 
the new patterns?

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestArrays.java line 2174:

> 2172:     // Same as test85 but with value class array
> 2173:     @Test
> 2174:     @IR(applyIf = {"ValueTypeReturnedAsFields", "true"},

Wrong rewrite? Should be `ValueTypePassFieldsAsArgs`? Same in another `@IR` 
rule below.

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestFieldNullMarkers.java line 
246:

> 244:  *                               -XX:+UnlockExperimentalVMOptions
> 245:  *                               -XX:+UseNullableAtomicValueFlattening 
> -XX:+UseNullFreeAtomicValueFlattening -XX:+UseNullFreeNonAtomicValueFlattening
> 246:  *                               -XX:+ValueTypePassFieldsAsArgs 
> -XX:+ValueTypeReturnedAsFields

Semantic config change: `ReturnedAsFields` -> `PassFieldsAsArgs`? This one 
seems to match the `@test id=all-flattening-pass-fields`. But is it a separate 
test bug?

test/hotspot/jtreg/compiler/valhalla/valuetypes/TestFieldNullMarkers.java line 
288:

> 286:  *                               -XX:+UnlockExperimentalVMOptions
> 287:  *                               -XX:+UseNullableAtomicValueFlattening 
> -XX:+UseNullFreeAtomicValueFlattening -XX:+UseNullFreeNonAtomicValueFlattening
> 288:  *                               -XX:+ValueTypePassFieldsAsArgs 
> -XX:-ValueTypeReturnedAsFields

This one is the opposite direction: `PassFieldsAsArgs`, but test ID is 
`all-flattening-return-fields`? Something is off here.

test/hotspot/jtreg/runtime/cds/appcds/dynamicArchive/HelloDynamicInlineClass.java
 line 42:

> 40: import jdk.test.lib.helpers.ClassFileInstaller;
> 41: 
> 42: public class HelloDynamicValueClass extends DynamicArchiveTestBase {

Same problem: class name changed, but file name was not. javac would probably 
reject it.

test/hotspot/jtreg/runtime/cds/appcds/test-classes/HelloInlineClassApp.java 
line 28:

> 26: import jdk.internal.vm.annotation.NullRestricted;
> 27: 
> 28: public class HelloValueClassApp {

The class name was changed to `HelloValueClassApp`, but the file is still 
`HelloInlineClassApp`. It would not build?

test/hotspot/jtreg/runtime/valhalla/valuetypes/ObjectMethods.java line 157:

> 155:             throw new RuntimeException("monitorenter did not fail");
> 156:         }
> 157:         // synchronized method modifiers tested by "BadValueTypes" CFP 
> tests

Missed the spot one line below: "InlineWithJni" -> "ValueWithJni"?

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

Changes requested by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/32673#pullrequestreview-5109320722
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931049021
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931018282
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3930995193
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3930995471
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931020903
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931010151
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931031895
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931035737
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931003013
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3930999776
PR Review Comment: https://git.openjdk.org/jdk/pull/32673#discussion_r3931046085

Reply via email to