[vector] RFC: Add scalable shapes for Arm Scalable Vector Extension

Tue Oct 30 17:40:14 UTC 2018

Thanks, Ningsheng.

> I have separated vectorapi.webrev01 into three patches. Now they are:
> 
> The scalable vector related changes:
> http://cr.openjdk.java.net/~njian/vectorapi/01-scalable-vector.webrev0/
> 
> Vector intrinsics related changes:
> http://cr.openjdk.java.net/~njian/vectorapi/02-vector-intrinsics.webrev0/

What I had in mind is getting VectorIntrinsics.reinterpret/cast() and 
all related changes (jdk + hotspot) separated, to focus on 
*ScalableVector shapes in a separate webrev.

Sorry for the confusion.

> AArch64 specific changes:
> http://cr.openjdk.java.net/~njian/vectorapi/03-aarch64-sve.webrev0/

Looks fine. It looks like a stand-alone change ready to be pushed. Do 
you agree?

Best regards,
Vladimir Ivanov

> 
> And the tests (the same as previous version):
> http://cr.openjdk.java.net/~njian/vectorapi/04-tests-scalable-vector.webrev0/
> 
> Could you please help to take a look?
> 
> Thanks,
> Ningsheng
> 
>> -----Original Message-----
>> From: Halimi, Jean-Philippe <jean-philippe.halimi at intel.com>
>> Sent: Tuesday, October 30, 2018 8:55 AM
>> To: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; Ningsheng Jian (Arm
>> Technology China) <Ningsheng.Jian at arm.com>; panama-dev at openjdk.java.net
>> Cc: nd <nd at arm.com>
>> Subject: RE: [vector] RFC: Add scalable shapes for Arm Scalable Vector Extension
>>
>> I do not have more comments, tests look good to me.
>>
>> Jp
>>
>> -----Original Message-----
>> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
>> Sent: Monday, October 29, 2018 4:16 PM
>> To: Ningsheng Jian (Arm Technology China) <Ningsheng.Jian at arm.com>;
>> panama-dev at openjdk.java.net; Halimi, Jean-Philippe <jean-
>> philippe.halimi at intel.com>
>> Cc: nd <nd at arm.com>
>> Subject: Re: [vector] RFC: Add scalable shapes for Arm Scalable Vector Extension
>>
>> Ningsheng,
>>
>>> http://cr.openjdk.java.net/~njian/sve/vectorapi.webrev01/
>>> and the test cases:
>>> http://cr.openjdk.java.net/~njian/sve/vectorapi-test.webrev01/
>>>
>>> All the vector api cases passed on my x86 machines with and without
>> VectorApiIntrinsics. There are some failures on AArch64, but they are not related
>> to this patch and we will fix them in follow-up patches.
>>
>> vectorapi-test.webrev01 looks good. Jp, do you have any comments?
>>
>> Regarding vectorapi.webrev01, please, separate it in 3 patches:
>>     (1) VectorIntrinsics-related changes
>>     (2) ScalableVector-related changes
>>     (3) aarch64-specific changes
>>
>> IMO #3 is fine, but I'd like to hear from Intel folks about #1 & #2.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>>
>>> Please help to review the changes.
>>>
>>> Thanks,
>>> Ningsheng
>>>
>>>
>>>> -----Original Message-----
>>>> From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
>>>> Sent: Friday, October 12, 2018 11:18 AM
>>>> To: Ningsheng Jian (Arm Technology China) <Ningsheng.Jian at arm.com>;
>>>> panama-dev at openjdk.java.net
>>>> Cc: nd <nd at arm.com>
>>>> Subject: Re: [vector] RFC: Add scalable shapes for Arm Scalable
>>>> Vector Extension
>>>>
>>>>
>>>>
>>>>>>>> So, my question boils down to: how vector shape checks are
>>>>>>>> expected to work on *ScalableVectors in presence of operations
>>>>>>>> which change vector
>>>>>> size.
>>>>>>>>
>>>>>>>> For example, IntScalableVector.rebracket() => LongScalableVector
>>>>>>>> which is fine, but ISV.resize() => ISV is what I'm concerned
>>>>>>>> about and existing checks aren't enough for *ScalableVectors
>>>>>>>> unless you check their length at runtime.
>>>>>>>
>>>>>>> Thanks for the detailed explanation. So, which checks do you mean
>>>>>>> here? Do
>>>>>> you mean this [1]? I find that current Java implementations already
>>>>>> has some runtime length checks. One thing we need to consider is
>>>>>> the ambiguity between existing sizes and scalable size. Current
>>>>>> VectorIntrinsics.reinterpret() just tries to get the shape from
>>>>>> size
>>>>>> (library_call.cpp:get_exact_klass_for_vector_box) instead of
>>>>>> getting the real class from input argument, which will lead to
>>>>>> ClassCastException. Maybe that can be fixed by adding real klass
>>>>>> type to the
>>>> reinterpret function.
>>>>>>
>>>>>> The checks I referred to are the casts on vector shapes which are
>>>>>> part of every vector operation (e.g., [2]), but they don't
>>>>>> immediately relate to shape-changing operations.
>>>>>>
>>>>>
>>>>> OK, but I would expect the normal operations between SV and other
>>>>> vectors
>>>> are illegal, though they may have the same size, since users usually
>>>> don't know the real size of SV. We should treat SV different from other
>> vectors.
>>>>
>>>> Yes, the checks currently in place effectively forbid mixing vectors
>>>> of different shapes. But API allows to reshape/cast vectors to
>>>> arbitrary shapes and that may cause problems if equivalent shapes meet at
>> runtime.
>>>>
>>>> Since concrete vector shapes aren't part of API, I believe it is
>>>> possible to exclude duplicating shapes at runtime. For example, SVs
>>>> can be used as a substitute for *512V shapes on 512-bit SVE or
>>>> AVX512-capable hardware. It seems current JDK- JVM interface
>>>> (VectorIntrinsics) should fit SV purposes well (both on x86 & ARM),
>>>> but it would be very interesting to hear your experience.
>>>>
>>>> API allows to query vectors of smaller/larger sizes than hardware
>>>> natively supports and implementation has to support that (even if
>> performance suffers).
>>>> It means SV shapes can coexist at runtime with existing shapes
>>>> (*64V/.../*512V) and casts/reshapes between all those shapes should work.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>>>> It seems I had some wrong assumptions when coming up with the examples.
>>>>>> Sorry for the confusion. Taking those into account, here's my take
>>>>>> on current state of vector shape changing operations w.r.t. SV shapes.
>>>>>>
>>>>>> There'll be 30 vector shapes in total (5 sizes x 6 element types):
>>>>>>       * Byte64V, Byte128V, Byte256V, Byte512V, ByteSV
>>>>>>       * Short64V, ..., ShortSV
>>>>>>       ...
>>>>>>       * Double64V, Double128V, ... Double512V, DoubleSV
>>>>>>
>>>>>> Depending on hardware, SVs may alias with "explicitly sized" shapes
>>>>>> (e.g., IntSV is equivalent to Int512V on AVX512 capable H/W).
>>>>>>
>>>>>> (1) Vector.rebracket(): works fine for SVs since vector size (in
>>>>>> bits) stays the
>>>> same:
>>>>>> IntSV <-> LongSV <-> ... <-> DoubleSV.
>>>>>>
>>>>>> (2) Vector.resize(): SV->SV doesn't make sense and the operation
>>>>>> should involve both SV and existing vector shapes: IntSV <->
>>>>>> Int64V/Int128V/.../Int512V. The ambiguity you mention can cause
>>>>>> problems and needs to be carefully handled to avoid 2 equivalent
>>>>>> shapes to meet at runtime. Otherwise, vector shape checks I
>>>>>> mentioned
>>>> earlier should be changed.
>>>>>>
>>>>>
>>>>> Yes, we will try to address this issue carefully in the follow-up webrev.
>>>>>
>>>>>> (3) Vector.cast(): SV->SV, SV<->64V/.../512V
>>>>>>         * size-preserving transformations (int <-> float):
>>>>>>           SV->SV work fine: IntSV <-> FloatSV, LongSV <-> DoubleSV;
>>>>>>
>>>>>>         * widening casts (e.g., int->long)
>>>>>>           SV->SV/64V/...:  truncate upper part of the vector, since
>>>>>> SVs already represent widest vector shapes;
>>>>>>           64V/...->SV: either truncated or filled with defaults
>>>>>> depending on actual size
>>>>>>
>>>>>>         * narrowing casts (e.g., long->int)
>>>>>>           SV->SV/64V/...: fill upper part of SV with defaults;
>>>>>>           64V/...->SV: either truncated or filled with defaults
>>>>>> depending on actual size
>>>>>>
>>>>>
>>>>> Thanks,
>>>>> Ningsheng
>>>>>