On Thu, 23 Apr 2026 11:26:26 GMT, Paul Hübner <[email protected]> wrote:

>> Casper Norrbin has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   feedback fixes
>
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/InheritedFlatFieldTest.java 
> line 33:
> 
>> 31: 
>> 32: /*
>> 33:  * @test
> 
> Is there merit enough to keep this test around for non-debug builds? I'm 
> asking because `assert_post_construction_invariants` is defined for debug 
> VMs.  I think it's probably fine to keep.

I think there's still value in just testing through the asserts in the test.

> test/hotspot/jtreg/runtime/valhalla/inlinetypes/InheritedFlatFieldTest.java 
> line 41:
> 
>> 39:  * @run main/othervm/native --enable-native-access=ALL-UNNAMED
>> 40:  *                          -XX:+UnlockDiagnosticVMOptions
>> 41:  *                          -XX:+UseFieldFlattening
> 
> If you're explicitly enabling `UseFieldFlattening`, you might want to enable 
> `UseNullFreeXXValueFlattening` where `XX` is the appropriate atomicity 
> (atomic here, I believe).

I went with Frederic's idea instead of removing the flattening asserts so we 
can run this in any configuration.

> test/hotspot/jtreg/runtime/valhalla/inlinetypes/libInheritedFlatFieldTest.c 
> line 26:
> 
>> 24: #include <jni.h>
>> 25: 
>> 26: #define VALUE_SIGNATURE 
>> "Lruntime/valhalla/inlinetypes/InheritedFlatFieldTest$Value;"
> 
> Nit: undefine these after you're done.

Fixed now.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2355#discussion_r3131709333
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2355#discussion_r3131701327
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2355#discussion_r3131710104

Reply via email to