[vector-api] Test patterns for conversion APIs.

[Bhateja, Jatin]

Hi Paul,

Thanks, updated patch has been placed at following link.

http://cr.openjdk.java.net/~jbhateja/vectorIntrinsics/TESTS/conversion-reinterpret-api/webrev.03/

Regards,
Jatin

> -----Original Message-----
> From: Paul Sandoz <paul.sandoz at oracle.com>
> Sent: Tuesday, November 19, 2019 2:00 AM
> To: Bhateja, Jatin <jatin.bhateja at intel.com>
> Cc: panama-dev at openjdk.java.net
> Subject: Re: [vector-api] Test patterns for conversion APIs.
> 
> Hi Jatin,
> 
> Apologies for the delay, I was busy last week.
> 
> I see you created a single class with all tests, I presumed you would create a
> template and a class per shape.  Either is fine by ne although separate
> generated classes do allow an easier scope for test selection and possible
> modification.
> 
> As to the new static constants: I see the value of them, but I am unsure of the
> names.  If you wanna proceed more quickly perhaps make them internal for
> now and follow up later on?
> 
> These new constants are really about copying rather than casting from
> different shapes.  That’s reflected in the symbol name when one create an
> equivalent conversion:
> 
> VectorOperators.Conversion<Integer, Integer> c =
>         VectorOperators.Conversion.ofReinterpret(int.class, int.class);
> 
> Or
> 
> VectorOperators.Conversion<Integer, Integer> c =
>         VectorOperators.Conversion.ofCast(int.class, int.class);
> 
> Either of which returns the same conversion instance with the symbol name
> “COPY_I2I”, and a kind of ‘I'
> 
> We should ensure the static field names correspond with the appropriate
> symbol name, and the arguments passed to the internal
> ConversionImpl.convert method are correct and equivalent explicit
> construction using the public static methods on Conversion.
> 
> Paul.
> 
> > On Nov 11, 2019, at 3:41 AM, Bhateja, Jatin <jatin.bhateja at intel.com>
> wrote:
> >
> > Hi Paul,
> >
> > Thanks for going through the patch, I have placed an updated revision
> covering your comments at following link.
> >
> > http://cr.openjdk.java.net/~jbhateja/vectorIntrinsics/TESTS/conversion
> > -reinterpret-api/webrev.02/
> >
> > For additional public VectorOperations, since APIs like convertShape
> > does allow a conversion b/w same type but different species hence it will
> add the needed convenience.
> >
> > Scope of this patch is limited to creation of unit tests which
> > exhaustively covers various test points for different species, types, and parts
> combinations for various conversion APIS.
> >
> > Best Regards,
> > Jatin
> >
> >> -----Original Message-----
> >> From: Paul Sandoz <paul.sandoz at oracle.com>
> >> Sent: Friday, November 8, 2019 11:52 PM
> >> To: Bhateja, Jatin <jatin.bhateja at intel.com>
> >> Cc: panama-dev at openjdk.java.net
> >> Subject: Re: [vector-api] Test patterns for conversion APIs.
> >>
> >> Hi Jatin,
> >>
> >> This is a good start, nice work.
> >>
> >> In VectorOperations you have added a list of identity conversion operators.
> >> Do you think these are really required to be part of the public API?
> >> Equivalent ops can be obtained from say
> >> VectorOperators.Conversion.ofCast(byte.class,
> >> byte.class) etc for testing purposes.
> >>
> >> Would it be possible split out the conversion unit tests into
> >> separate files, since a significant number of tests have been added?
> >> (Likewise, I think any future bound check failure tests for
> >> load/store should probably be in separate files).
> >>
> >> For the unit test name I recommend sticking to the Java method name e.g.
> >> convertB2S which fits more closely with the existing naming convention.
> >> (I suspect we can clean up the suffix <Bits>VectorTests, of method
> >> names for other tests, since its part of the class name).
> >>
> >> The generated JMH tests look like work in progress? I don’t think
> >> they will compile e.g. there are a lot of generated methods with the
> >> same name and signature.  Also consider separating out into a separate file.
> >>
> >> For the JMH test name swap the method name and conversion op
> >> s/B2Bconvert/convertB2B, then it's in sync with the unit test method
> >> name, but it becomes a little easier to select tests with a regex.
> >>
> >>
> >> I took a brief look at more of the test code itself, generally well
> >> structured, I’ll take a closer look on another round of this webrev.
> >>
> >> Thanks,
> >> Paul.
> >>
> >>
> >>
> >>> On Nov 8, 2019, at 5:01 AM, Bhateja, Jatin <jatin.bhateja at intel.com>
> wrote:
> >>>
> >>> Hi All
> >>>
> >>> Please find below a link to patch adding test patterns for 4 conversion
> APIs.
> >>>
> >>> - convert
> >>> - convertShape
> >>> - castShape
> >>> - reinterpretShape
> >>>
> >>> http://cr.openjdk.java.net/~jbhateja/vectorIntrinsics/TESTS/conversi
> >>> on-
> >> reinterpret-api/webrev.01/
> >>>
> >>> Kindly review and share your feedback.
> >>>
> >>> Regards
> >>> Jatin
> >