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

Wed Oct 31 10:19:40 UTC 2018

Thank you Vladimir!

I have restructured the webrevs as follows:

1) VectorIntrinsics reinterpret/cast changes:
http://cr.openjdk.java.net/~njian/vectorapi/01-vectorintrinsics-reinterpret.webrev1/

2) Scalable vector related changes:
http://cr.openjdk.java.net/~njian/vectorapi/02-scalable-vector.webrev1/

3) AArch64 specific changes:
http://cr.openjdk.java.net/~njian/vectorapi/03-aarch64-sve.webrev1/
This is the same as previous version, and yes, this is a standalone change and can be applied and pushed cleanly. I am OK to push this first. Thanks!

4) Tests (the same as previous version):
http://cr.openjdk.java.net/~njian/vectorapi/04-tests-scalable-vector.webrev1/

Thanks,
Ningsheng

> -----Original Message-----
> From: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
> Sent: Wednesday, October 31, 2018 1:40 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
> 
> 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.webre
> > v0/
> 
> 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.w
> > ebrev0/
> >
> > 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
> >>>>>