[vector] refactoring api for mask, shuffle creation
Brian Goetz
brian.goetz at oracle.com
Sat Apr 20 18:05:07 UTC 2019
I think the API will be pretty much there at that point.
I think that the API _specs_ could use a good scrubbing first; it's been
a while since these were reviewed, and some things have been shuffled
around, and we've not really scrutinized them at the level that we
should before going to CSR, because we were still trying to make things
work and find the right API. Now that we've cleared that hurdle, I
think spending some quality time with the specifications will be a good
time investment.
I think the "long pole" for getting to PTT will be getting the JIT code
reviewed.
On 4/19/2019 12:52 PM, Viswanathan, Sandhya wrote:
>
> Hi Brian,
>
> Thanks a lot for all your guidance through this. We plan to
> incorporate the rest of the comments in next couple of days and update
> the JEP accordingly. Does requesting a CSR review at 80% sometime next
> week sound like a reasonable target?
>
> Best Regards,
>
> Sandhya
>
> *From:*Brian Goetz [mailto:brian.goetz at oracle.com]
> *Sent:* Friday, April 19, 2019 4:27 AM
> *To:* Kharbas, Kishor <kishor.kharbas at intel.com>
> *Cc:* panama-dev at openjdk.java.net; John Rose <john.r.rose at oracle.com>;
> Vladimir Ivanov <vladimir.x.ivanov at oracle.com>; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> *Subject:* Re: [vector] refactoring api for mask, shuffle creation
>
> Great. I sent you some minor API comments yesterday, but otherwise I
> am happy with where the API has landed. This is a much more compact
> and more approachable API than we started with.
>
>
>
> On Apr 16, 2019, at 8:22 PM, Kharbas, Kishor
> <kishor.kharbas at intel.com <mailto:kishor.kharbas at intel.com>> wrote:
>
> Hi Brian,
>
> Switching class Species to an interface does not perturb code
> generation. In the updated patch I have made this change along
> with moving the new fields to AbstractSpecies.
>
> New bug created :https://bugs.openjdk.java.net/browse/JDK-8222584
>
> webrev
> :http://cr.openjdk.java.net/~kkharbas/vector-api/8222584/webrev.01/
>
> If the patch looks good, I will go ahead and push this.
>
> Thanks,
>
> Kishor
>
> *From:*Brian Goetz [mailto:brian.goetz at oracle.com]
> *Sent:*Monday, April 15, 2019 10:46 AM
> *To:*Kharbas, Kishor <kishor.kharbas at intel.com
> <mailto:kishor.kharbas at intel.com>>
> *Cc:*panama-dev at openjdk.java.net
> <mailto:panama-dev at openjdk.java.net>; John Rose
> <john.r.rose at oracle.com <mailto:john.r.rose at oracle.com>>; Vladimir
> Ivanov <vladimir.x.ivanov at oracle.com
> <mailto:vladimir.x.ivanov at oracle.com>>; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com <mailto:sandhya.viswanathan at intel.com>>
> *Subject:*Re: [vector] refactoring api for mask, shuffle creation
>
> This would also be a good time to see if the generated code is
> perturbed by switching Species to an interface. Since all the
> actual Species instances are typed more sharply (e.g.,
> IntSpecies), I think we might be able to get away with this? If
> we can do so now, that’s a win, as its one less thing we will be
> tempted to change later.
>
> In any case, the behavior you are adding here (the fields, and the
> constructor) should be live in AbstractSpecies, where the rest of
> the state is. We definitely don’t want to be splitting this
> across two classes.
>
>
>
>
> Thanks Brian for the feedback. I also like the idea of
> renaming Shape, Species, etc. Code generation is not affected
> as per my testing so far, I will follow up with more testing.
>
> Just wanted to make sure changes to Species class (copied
> below) do not undo the efforts been put to make it value friendly.
>
> * @param <E> the boxed element type of this species
>
> */
>
> public static abstract class Species<E> {
>
> - Species() {}
>
> +
>
> + @FunctionalInterface
>
> + interface fShuffleFromArray<E> {
>
> + Shuffle<E> apply(int[] reorder, int idx);
>
> + }
>
> +
>
> + final Function<boolean[], Mask<E>> maskFactory;
>
> + final Function<IntUnaryOperator, Shuffle<E>>
> shuffleFromOpFactory;
>
> + final fShuffleFromArray<E> shuffleFromArrayFactory;
>
> +
>
> + Species(Function<boolean[], Vector.Mask<E>> maskFactory,
>
> + Function<IntUnaryOperator, Shuffle<E>> shuffleFromOpFactory,
>
> + fShuffleFromArray<E> shuffleFromArrayFactory) {
>
> + this.maskFactory = maskFactory;
>
> + this.shuffleFromOpFactory = shuffleFromOpFactory;
>
> + this.shuffleFromArrayFactory = shuffleFromArrayFactory;
>
> + }
>
> /**
>
> * Returns the primitive element type of vectors
> produced by this
>
> @@ -1100,6 +1120,18 @@
>
> Shape s = Shape.forBitSize(vectorBitSize);
>
> return Species.of(c, s);
>
> }
>
> +
>
> + interface FOpm {
>
> + boolean apply(int i);
>
> + }
>
> +
>
> + Vector.Mask<E> opm(Vector.Species.FOpm f) {
>
> + boolean[] res = new boolean[length()];
>
> + for (int i = 0; i < length(); i++) {
>
> + res[i] = f.apply(i);
>
> + }
>
> + return maskFactory.apply(res);
>
> + }
>
> }
>
> Thanks
>
> Kishor
>
> *From:*Brian Goetz [mailto:brian.goetz at oracle.com]
> *Sent:*Friday, April 12, 2019 10:18 AM
> *To:*Kharbas, Kishor <kishor.kharbas at intel.com
> <mailto:kishor.kharbas at intel.com>>;panama-dev at openjdk.java.net
> <mailto:panama-dev at openjdk.java.net>
> *Cc:*John Rose <john.r.rose at oracle.com
> <mailto:john.r.rose at oracle.com>>; Vladimir Ivanov
> <vladimir.x.ivanov at oracle.com
> <mailto:vladimir.x.ivanov at oracle.com>>; Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com
> <mailto:sandhya.viswanathan at intel.com>>
> *Subject:*Re: [vector] refactoring api for mask, shuffle creation
>
> Moving the classes to top-level is a slam-dunk. There's no
> need for nesting in this API. We might ask ourselves later if
> we should rename some of these (Shape -> VectorShape) but
> that's easy if we decide to do so.
>
> I like the idea of moving the mask/shuffle creation methods to
> Mask/Shuffle, since they will be easier for developers to find
> (and secondarily it reduces the surface area of the API.) Now,
> the Mask and Shuffle abstractions effectively stand on their
> own; they are their own top-level entity, and they take
> control of their own creation. This results in a simpler API.
>
> I hope that the generated code remains good with this shift.
>
>
>
> On 4/11/2019 8:07 PM, Kharbas, Kishor wrote:
>
> Hi,
>
> I am experimenting with few changes to improve the API.
> Specifically,
>
> 1.mask and shuffle creation methods (example,
> maskFromArray()) right now are defined in XxxVector. From
> user’s perspective it seems more intuitive for
> Mask/Shuffle to define these methods. This patch moves
> them out of XxxVector to their respective classes.
>
> 2.Moved classes – Species, Shape, Shuffle and Mask, at the
> top-level instead of being nested inside vector.
>
> Please help me evaluate the changes. Apart from moving
> methods around and changing signatures, there are changes
> in Species class -- addition of factory methods to create
> masks and shuffles.
>
> I am in the process of testing the patch.
>
> Webrev
> :http://cr.openjdk.java.net/~kkharbas/vector-api/species_refactoring-phase2/webrev-phase2.02/
>
> Javadoc
> :http://cr.openjdk.java.net/~kkharbas/vector-api/species_refactoring-phase2/webrev_phase-2.JavaDoc.02/jdk.incubator.vector/jdk/incubator/vector/package-summary.html
>
> Thanks
>
> Kishor
>
More information about the panama-dev
mailing list