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

Tue Oct 30 10:58:12 UTC 2018

Thank you Vladimir and Jp for the comments!

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/

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

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
> >>>