Flat Array null check issue - Re: RFR(L) Valhalla support for AArch64 v.05

[Dmitry Samersoff]

Hello Rolland, Ioi

I narrowed the problem (below) to:

GraphKit::gen_value_array_null_guard

and following replacement of

GetNullFreeProperty in compile.cpp


Interpreter in a similar place uses the code (below):

  ldrw(dst, Address(src, oopDesc::klass_offset_in_bytes()));
  asrw(dst, dst, oopDesc::narrow_storage_props_shift);

  andr(dst, dst, ArrayStorageProperties::null_free_value);
  cbnz(dst, is_null_free_array);


but C2 does:

   # LoadNKlass node
   ldrw  $dst, $mem
   andr  $dst, $dst, oopDesc::compressed_klass_mask()

   #AndINode
   andr  $dst, 1 << (oopDesc::narrow_storage_props_shift  +
                     ArrayStorageProperties::null_free_bit);

   # cmpI
   cbz(temp_reg, is_null_free_array);


Is it correct?
Do you have any ideas how to go forward with it?


-Dmitry


On 05.07.19 08:14, Ioi Lam wrote:
> So the AArch64 port fails but the x64 port passes? I just tested on x64
> and the test passed for me.
> 
> I would suggest running both ports side-by-side with gdb, and see when
> the behavior starts to diverge.
> 
> To help debugging, you can try to strip down TestFlatArrayThreshold.java
> to the minimum and also pass -XX:CompileCommand=compileonly,*::test2
> -XX:CompileCommand=print,*::test2 to the @run line.
> 
> Thanks
> - Ioi
> 
> On 7/4/19 11:01 AM, Dmitry Samersoff wrote:
>> Ioi,
>>
>> After last rebase the test TestFlatArrayThreshold start failing:
>>
>> java.lang.RuntimeException: NullPointerException expected
>>          at TestFlatArrayThreshold.main(TestFlatArrayThreshold.java:128)
>>
>> i.e. when we pass null to:
>>
>>   public static MyValue1? test2(MyValue1?[] va, MyValue1? vt) {
>>          va[0] = vt;
>>          return va[1];
>>      }
>>
>> It fails if ValueKlass::flatten_array() returns false,
>> but passes if it returns true. It also passes wit -Xint
>>
>> Do you know where I should look for the problem? I didn't find any
>> obvious platfrom-specific changes.
>>
>> -Dmitry
>>
>> On 27.06.2019 19:09, Ioi Lam wrote:
>>> Hi Dmitry,
>>>
>>> During the port, have you found anything that should be refactored and
>>> put into shared code?
>>>
>>> One thing that I am thinking of doing is to refactor the code in
>>> macroAssembler_x86.cpp that deals with ValueTypePassFieldsAsArgs.
>>> Hopefully that will make the port to other architectures easier.
>>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>> On 6/27/19 12:43 AM, Dmitry Samersoff wrote:
>>>> Hello Everybody,
>>>>
>>>>
>>>> Please review v.05 of adding AArch64 support to lworld branch
>>>>
>>>> http://cr.openjdk.java.net/~dsamersoff/valhalla_lworld_aarch64/webrev.05/
>>>>
>>>>
>>>> What was done:
>>>>
>>>>     Implemented Valhalla support for aarch64.
>>>>
>>>>     Support for ValueTypePassFieldsAsArgs and ValueTypeReturnedAsFields
>>>>     is not completed and is not included into this webrev
>>>>     to reduce review and rebasing efforts.
>>>>
>>>> Testing:
>>>>
>>>>      All tests in runtime/valhalla/valuetypes and in
>>>> compiler/valhalla/valuetypes/ are passed
>>>>
>>>>      compiler/valhalla/valuetypes/TestMethodHandles.java
>>>>      compiler/valhalla/valuetypes/TestBasicFunctionality.java
>>>>
>>>>      are passed after manually tweaking of output patterns, I have not
>>>> found a way to support both x86 and aarch64, so these tests remain
>>>> unchanged and run on x64 only.
>>>>
>>>>
>>>> -Dmitry
>