[vector] Intrinsics for resize and tests

[Lupusoru, Razvan A]

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/

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