[lworld] RFR: 8264414: [lworld] [AArch64] TestBufferTearing.java fails with C1 [v2]

[Tobias Hartmann]

On Fri, 9 Apr 2021 06:33:18 GMT, Nick Gasson <ngasson at openjdk.org> wrote:

>> We see failures like this on AArch64 when MyValue.incrementAndCheck() is
>> compiled with C1:
>> 
>>   java.lang.RuntimeException: Inconsistent field values: expected 0 to equal 675128
>>         at jdk.test.lib.Asserts.fail(Asserts.java:594)
>>         at jdk.test.lib.Asserts.assertEquals(Asserts.java:205)
>>         at jdk.test.lib.Asserts.assertEQ(Asserts.java:178)
>>         at compiler.valhalla.inlinetypes.MyValue.incrementAndCheck(TestBufferTearing.java:81)
>>         at compiler.valhalla.inlinetypes.TestBufferTearing$Runner.run(TestBufferTearing.java:124)
>> 
>> The barrier that is usually inserted on return from a method that wrote
>> final fields should be sufficient to prevent another thread seeing the
>> zero-initialised intermediate state.  However this barrier isn't
>> inserted at the moment because method()->is_object_constructor() is
>> false for primitive class constructors.
>> 
>> C2 has a similar guard around the memory barrier in Parse::do_exits().
>> I'm not sure if that needs amending as well but I've not seen any
>> failures due to it.
>
> Nick Gasson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Two more missing membars

Very nice that you were able to come up with a regression test!

Regarding the combination of `InlineFieldMaxFlatSize` and `FlatArrayElementMaxSize`, couldn't you achieve the same result by using an Object or Interface array/field to prevent flattening?

The fix looks good to me but I think we also need membars in `GraphBuilder::access_field` (there are two places where we create a buffer via `new NewInlineTypeInstance`). Would be great if you could add a regression test for that as well.

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

PR: https://git.openjdk.java.net/valhalla/pull/376