[vector] Vector API -- alignment with value types
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Tue Apr 9 22:07:02 UTC 2019
Looks good!
Best regards,
Vladimir Ivanov
On 09/04/2019 13:40, Kharbas, Kishor wrote:
> Hi,
>
> Brian and I have made further changes to Sandhya's patch towards making the api amenable to value types and also simplify the api. Summary of changes are,
> 1. Change the species hierarchy, essentially making it flatter. We removed definition of XxxNnnSpecies classes (e.g. Float256Species).
> 2. Removed the only remaining public method in XxxSpecies after Sandhya's patch (zero()). XxxVector already defines zero() which can be used.
> 3. Since XxxSpecies does not have any more public methods, it is made package private. User accessible Species class is Vector.Species<E>. Static constants in XxxVector are now of type Species<Xxx> and can be accessed directly by users. The api methods which took XxxSpecies as input have been changed to take Species<Xxx>.
> 4. Modified jtreg tests and performance benchmarks to work with the changes. Tests continue to pass and made sure intrinification continues to happen. (Added @Stable annotation to static final fields (example, AbstractSpecies.boxType) which replaced constants return by say Float256Species.boxType())
>
> Webrev : http://cr.openjdk.java.net/~kkharbas/vector-api/species_refactoring-phase2/webrev-phase2.01/
> Java-doc : http://cr.openjdk.java.net/~kkharbas/vector-api/species_refactoring-phase2/webrev_phase-2.JavaDoc.01/jdk.incubator.vector/module-summary.html
>
> Please let me know whether the changes look ok to be pushed.
>
> Thanks,
> Kishor
>
>> -----Original Message-----
>> From: panama-dev [mailto:panama-dev-bounces at openjdk.java.net] On
>> Behalf Of Viswanathan, Sandhya
>> Sent: Wednesday, March 6, 2019 11:37 AM
>> To: Brian Goetz <brian.goetz at oracle.com>
>> Cc: panama-dev at openjdk.java.net
>> Subject: RE: [vector] Vector API -- alignment with value types
>>
>>
>> I have enclosed below the webrev which moves the rest of the identified
>> methods away from Species, namely (broadcast, random, scalars, single,
>> cast(Mask), cast(shuffle)):
>> http://cr.openjdk.java.net/~sviswanathan/vectorIntrinsics/Brian/zero_phas
>> e2/
>>
>> All the tests continue to pass with these changes.
>>
>> Please let me know if this looks ok and if I can go ahead and push these
>> changes.
>>
>> Also looking for advice on next steps to get ready for API review.
>>
>> Best Regards,
>> Sandhya
>>
>>
>> -----Original Message-----
>> From: Brian Goetz [mailto:brian.goetz at oracle.com]
>> Sent: Friday, February 22, 2019 8:39 AM
>> To: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
>> Cc: Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; John Rose
>> <john.r.rose at oracle.com>; panama-dev at openjdk.java.net
>> Subject: Re: [vector] Vector API -- alignment with value types
>>
>> This is looking good!
>>
>> Assuming we still like this direction (I do), the next step is to do the same
>> trick with Species.cast(Mask) and Species.cast(Shuffle), which will leave
>> Species with just the most basic methods:
>>
>> - data accessors
>> - zero (which eventually becomes a data accessor for a cached vector value)
>> - static factories for species
>>
>> And then, do the same with the hand-specialized methods in the specialized
>> Species, such as the following in ShortVector:
>>
>> - broadcast(short)
>> - single(short)
>> - random()
>> - scalars()
>>
>> (which is just more of the same trick). Then, we’re in a position to do the big
>> collapsing of Species we want to get to, where we turn Species into an
>> interface, and {Int,Long,Short,Float,Double}Species into enums.
>>
>> At this point, Species is now more “data” than “behavior”. To complete the
>> task of making species nearly invisible, we can add overloads of the static
>> methods we migrated from Species to XxxVector that do _not_ take a
>> leading species argument, and which default to the preferred species for
>> that type. Then most species-agnostic code can just do things like
>>
>> IntVector.zero()
>> IntVector.fromArray(…)
>>
>> and just operate on those.
>>
>> At that point, I think this transform is complete, and the Species objects are
>> ready to become values when we have values.
>>
>> Then we can talk about how Vector objects become values too!
>>
>>
>>
>>> On Feb 18, 2019, at 11:53 AM, Viswanathan, Sandhya
>> <sandhya.viswanathan at intel.com> wrote:
>>>
>>>
>>> Please find below the updated webrev where the fromArray methods are
>> moved from Species to XxxVector:
>>> http://cr.openjdk.java.net/~sviswanathan/vectorIntrinsics/Brian/zero/w
>>> ebrev.01/
>>>
>>> The tests and benchmark directories are also updated accordingly. All the
>> tests continue to pass with this webrev.
>>>
>>> In this webrev, I have also made the change suggested by Vladimir
>> regarding XxxVector.zero() method.
>>>
>>> Your review and feedback is welcome.
>>>
>>> After this the following public methods remain on Species and
>> $Type$Species:
>>> public abstract Class<E> elementType();
>>> public abstract int elementSize();
>>> public abstract Shape shape();
>>> public int length();
>>> public int bitSize();
>>> public static <E> Vector.Species<E> of(Class<E> c, Shape s);
>>> public static <E> Vector.Species<E> ofPreferred(Class<E> c);
>>> public abstract Vector<E> zero();
>>>
>>> public abstract <F> Mask<E> cast(Mask<F> m);
>>> public abstract <F> Shuffle<E> cast(Shuffle<F> s);
>>>
>>> public abstract $abstractvectortype$ broadcast($type$ e);
>>> public final $abstractvectortype$ single($type$ e);
>>> public $abstractvectortype$ random();
>>> public abstract $abstractvectortype$ scalars($type$... es); The
>>> last four could be a candidate to move to XxxVector. Please let me know
>> your thoughts.
>>>
>>> Best Regards,
>>> Sandhya
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
>>> Sent: Tuesday, February 12, 2019 10:23 AM
>>> To: Brian Goetz <brian.goetz at oracle.com>
>>> Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>;
>>> panama-dev at openjdk.java.net; John Rose <john.r.rose at oracle.com>
>>> Subject: Re: [vector] Vector API -- alignment with value types
>>>
>>>
>>>> Sure, for now. When vectors are values, we probably won’t need such
>> inspection?
>>>
>>> In general, I don't think it changes much with value types: special
>>> support on JIT-compiler side is still needed to strength-reduce vector
>>> construction operation. Otherwise, a memory load (even from a
>>> flattened
>>> representation) is needed to materialize a value unless JIT knows what
>>> the value is.
>>>
>>> But for particular case of Vector.zero() default value can be used
>>> (instead of caching).
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>>>> On Feb 11, 2019, at 8:12 PM, Vladimir Ivanov
>> <vladimir.x.ivanov at oracle.com> wrote:
>>>>>
>>>>>
>>>>>> I thought this might be the case; zeros are so common that we may
>>>>>> well want to just instantiate a zero vector as a species field,
>>>>>> which can be shared. Then we just expose “species.zero()” as a
>>>>>> method (which ByteVector.zero() can delegate to.)
>>>>>
>>>>> FTR zero() is not cached, but backed by an intrinsic, because it's easier to
>> optimize: it doesn't require JIT-compiler to inspect the boxed value to be
>> able to use faster idiom when constructing vector value at runtime (e.g., xor
>> vs load).
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>>> I didn’t want to do this in my first patch because I was mostly intent on
>> providing that we will get the same inlining with a more value-friendly API,
>> but this makes sense to me.
>>>>>
>>>>>>> On Feb 8, 2019, at 7:23 PM, Vladimir Ivanov
>> <vladimir.x.ivanov at oracle.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~sviswanathan/vectorIntrinsics/Brian/z
>>>>>>>> ero/webrev.00/
>>>>>>>
>>>>>>> I have one comment on implementation:
>>>>>>>
>>>>>>>
>> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.jav
>> a:
>>>>>>>
>>>>>>> + public static ByteVector zero(ByteSpecies species) {
>>>>>>> + return VectorIntrinsics.broadcastCoerced((Class<ByteVector>)
>> species.boxType(), byte.class, species.length(),
>>>>>>> + 0,
>>>>>>> + (z -> species.zero()));
>>>>>>> + }
>>>>>>>
>>>>>>> There's a slight change in behavior compared to "species.zero()" call
>> and that's because not all arguments to VectorIntrinsics.broadcastCoerced()
>> are guaranteed to be constant.
>>>>>>>
>>>>>>> If JIT can't devirtualize & inline "species.boxType()", intrinsification
>> fails and falls back to default implementation.
>>>>>>>
>>>>>>> In case of species.zero(), a failure to inline callee leads to a virtual call,
>> but callee contains intrinsified operation.
>>>>>>>
>>>>>>> Overall, I believe it shouldn't matter from performance perspective
>> right now, because a call breaks vector box elimination anyway, but in a
>> longer term it may become important.
>>>>>>>
>>>>>>> But I'd still prefer to see calls to VectorIntrinsics be fully specialized. In
>> the particular case I referred to, IMO it's better to just call species.zero().
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Vladimir Ivanov
>>>>
>
More information about the panama-dev
mailing list