[vector] Intrinsics for resize and tests
Paul Sandoz
paul.sandoz at oracle.com
Fri Apr 6 01:40:00 UTC 2018
> On Apr 5, 2018, at 6:25 PM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
>
> Hi Paul,
>
> I have addressed most of your comments and you can find updated patch here:
> http://cr.openjdk.java.net/~rlupusoru/panama/webrev_resize_04/
>
X-Vector.java
—
277 @Override
278 @ForceInline
279 public <T extends Shape> $abstractvectortype$<T> resize(Species<$Boxtype$, T> species) {
280 return ($abstractvectortype$<T>) species.reshape(this);
281 }
You removed this, but i think we need that for covariant overrides, just make it abstract and likewise on Vector?
The CovarOverrideTest test should catch that, if not it's an error in the test.
X-VectorBits.java.template
—
1293 throw new InternalError("Unimplemented size");
s/Unimplemented size/Unimplemented element type
VectorReshapeTests.java
—
Test looks great. I like the way the computation of the expected result simplified.
> Comments addressed:
> - Removed @ForceInline from reshape
> - Implemented Vector rebracket as part of species
> - Added rebracket tests
> - Updated NUM_ITER to be much larger (JP's property does not make sense anymore since in updated form it needs many iterations to compile)
It still does, if run with the interpreter or with the intrinsics disabled, so we test the Java versions. I envisage adding another @run to the jtreg configuration turning off the intrinsics and setting the iteration count to a lower-value.
No need for another webrev.
Paul.
> - Using your new species instance obtainer to avoid cast
> - Fixed BYTE generator to take actual size
> - No longer loop through array when doing resize
> - Avoided raw types
> - Got rid of invocation count
>
> I have managed to fix C2 error that was making the resize tests to fail. However, now the newly added rebracketing tests fail when vector intrinsics are enabled.
>
> Remaining tasks:
> - Root cause and fix rebracket failure
> - Add tests for resize and rebracket for masks as well
> - Add intrinsic for rebracket and resize for masks
>
> Let me know if you have any more comments.
>
> Thanks!
>
> --Razvan
>
> -----Original Message-----
> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> Sent: Wednesday, April 04, 2018 5:15 PM
> To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
> Cc: panama-dev at openjdk.java.net; Vladimir Ivanov <vladimir.x.ivanov at oracle.com>
> Subject: Re: [vector] Intrinsics for resize and tests
>
> HI Razvan,
>
>
>> On Apr 4, 2018, at 2:54 PM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
>>
>> Hi everyone,
>>
>> I have added the x86 implementation for resize and it generates pretty efficient code. In case of upsizing, it zeros out larger register and simply generates a move. In case of downsizing, it generates a simple move. In case of no size change, it generates nothing.
>> http://cr.openjdk.java.net/~rlupusoru/panama/webrev_resize_02/
>
> That’s impressive, i am surprised given the branching in the species resize implementation, but i guess the profiling prunes them, and hoists stuff out of loops.
>
> X-Vector.java.template
>
> 791 @Override
> 792 @ForceInline
> 793 public <F, T extends Shape> $abstractvectortype$<S> reshape(Vector<F, T> o) {
> 794 int blen = Math.max(o.species().bitSize(), bitSize()) / Byte.SIZE;
> 795 ByteBuffer bb = ByteBuffer.allocate(blen).order(ByteOrder.nativeOrder());
> 796 o.intoByteBuffer(bb, 0);
> 797 return fromByteBuffer(bb, 0);
> 798 }
>
> Is the @ForceInline required ?
>
> Do you plan later on to address optimizing the species implementation of reshape and rebracket? e.g. can we move the concrete vector.rebracket implementation to the corresponding concrete species rebracket implementation?
>
>
>>
>> I have also added appropriate tests for resize. Because C2 requires effectively monomorphic types for successful intrinsification, the species are not generated automatically but instead explicitly passed in. However, the tests currently fail due to an issue in C2 from merging phis of different vector sizes - and I need Vladimir’s assistance in root causing and fixing that issue. That said, if I comment out code and test only one type transition at a time, everything works correctly.
>>
>
> Nice, this test can serve as a pattern for reshape and rebracket tests.
>
> Just some minor stuff that could be addressed now if convenient or later.
>
>
> 19 static final int NUM_ITER = 20;
>
> We can reuse Jp’s system property.
>
>
> 21 static final IntVector.IntSpecies<Shapes.S64Bit> ispec64 = (IntVector.IntSpecies<Shapes.S64Bit>) Vector.speciesInstance(Integer.class, Shapes.S_64_BIT);
>
> I am making a mental note to add static a speciesInstance method to the subtypes so as to avoid the cast.
>
>
> 78 static final List<IntFunction<byte[]>> BYTE_GENERATORS = List.of(
> 79 withToString("byte[i * 5]", (int s) -> {
> 80 return fill(s * 1000,
> 81 i -> (byte)(i * 5));
> 82 }),
> 83 withToString("byte[i + 1]", (int s) -> {
> 84 return fill(s * 1000,
> 85 i -> (((byte)(i + 1) == 0) ? 1 : (byte)(i + 1)));
> 86 })
> 87 );
>
> Perhaps we should replace s * 1000 with just s, as it’s hard to understand what the actual size of the byte[] array will be. (Probably should revisit this same pattern in the other tests.)
>
>
> 97 static void testResize(Vector.Species a, Vector.Species b, byte[] input) {
>
> Make generic and avoid raw types.
>
>
> 110 for (int i = 0; i < loop_bound; i++) {
>
> Given we have data providers and the outer NUM_ITER loop i am not sure this loop is necessary. Is it better to simplify, remove this loop and just access the input from position 0?
>
>
> 128 @Test(dataProvider = "byteUnaryOpProvider", invocationCount = 2)
> 97 static void testResize(Vector.Species a, Vector.Species b, byte[] input) {
>
> invocationCount required?
>
> Paul.
>
>> Please let me know if you have any comments. Thanks!
>>
>> --Razvan
>
More information about the panama-dev
mailing list