[vector-api] Test patterns for conversion APIs.

[Paul Sandoz]

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/conversion-
>> reinterpret-api/webrev.01/
>>> 
>>> Kindly review and share your feedback.
>>> 
>>> Regards
>>> Jatin
>