VectorAPI Variable Shift intrinsics and Fix for AddReduction and Max

[Rukmannagari, Shravya]

Hi All,
Please find the updated patch with the suggested changes and also a fix for AddReduction and max intrinsics.
http://cr.openjdk.java.net/~srukmannagar/VectorAPI_shiftVar/webrev.03/

Thanks,
Shravya.

-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com] 
Sent: Friday, April 20, 2018 3:13 PM
To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>
Cc: panama-dev at openjdk.java.net
Subject: Re: VectorAPI Variable Shift intrinsics

Shravya,

> ..., but we currently do not support a vector node without a scalar node (and there would be no scalar variable shift). ...

Can you elaborate a bit here?

I had the following 2 cases in mind:

instruct vsll8L_reg(vecZ dst, vecZ src, vecS shift) %{
   predicate(UseAVX > 2 && n->as_Vector()->length() == 8);
   match(Set dst (LShiftVL src shift));

vs

instruct vsllv8L_reg_evex(vecZ dst, vecZ src, vecZ shift) %{
   predicate(UseAVX > 1 && n->as_Vector()->length() == 8 && 
n->in(2)->Opcode() != Op_LShiftCntV);
   match(Set dst (LShiftVL src shift));

vecS vs vecZ for shifts, but both are represented with LShiftVL node.


BTW spotted some inconsistencies in predicates:

+instruct vsllv8L_reg_evex(vecZ dst, vecZ src, vecZ shift) %{
+  predicate(UseAVX > 1 && n->as_Vector()->length() == 8 && 
n->in(2)->Opcode() != Op_LShiftCntV);

+instruct vsrlv16I_reg_evex(vecZ dst, vecZ src, vecZ shift) %{
+  predicate(UseAVX > 1 && n->as_Vector()->length() == 16 && 
n->in(2)->Opcode() != Op_RShiftCntV);

+instruct vsrav4L_reg_evex(vecY dst, vecY src, vecY shift) %{
+  predicate(UseAVX > 1 && n->as_Vector()->length() == 4 && 
n->in(2)->Opcode() != Op_RShiftCntV);

+instruct vsllv16I_reg_evex(vecZ dst, vecZ src, vecZ shift) %{
+  predicate(UseAVX > 1 && n->as_Vector()->length() == 16 && 
n->in(2)->Opcode() != Op_LShiftCntV);

Shouldn't it be UseAVX > 2?

Best regards,
Vladimir Ivanov

> 
> Thanks,
> Shravya.
> 
> -----Original Message-----
> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
> Sent: Friday, April 20, 2018 2:24 PM
> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>; Paul Sandoz <paul.sandoz at oracle.com>
> Cc: panama-dev at openjdk.java.net
> Subject: Re: VectorAPI Variable Shift intrinsics
> 
> Sharvya,
> 
> The usage of arity of vector operation to detect that shift value is a vector and not scalar looks a bit ugly.
> 
> IMO the need for such parameter is a sign of a problem (but in a different place): *ShiftV* nodes are used to represent both variable scalar & vector shifts.
> 
> I'd prefer to see different type of nodes representing those operations, but until we are there, I'm fine with the version you propose.
> 
> Best regards,
> Vladimir Ivanov
> 
> On 4/20/18 12:05, Rukmannagari, Shravya wrote:
>> Hi Paul,
>> I changed the name from meta to op_arity. Please find the modified patch below.
>> http://cr.openjdk.java.net/~srukmannagar/VectorAPI_shiftVar/webrev.02/
>>
>> Thanks,
>> Shravya.
>>
>> -----Original Message-----
>> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
>> Sent: Thursday, April 19, 2018 5:33 PM
>> To: Rukmannagari, Shravya <shravya.rukmannagari at intel.com>
>> Cc: panama-dev at openjdk.java.net
>> Subject: Re: VectorAPI Variable Shift intrinsics
>>
>> It looks good, but i find the parameter meta a little too opaque, perhaps “arity" or “op_arity" would be a better name? where a 0 value means don’t care or undefined?
>>
>> Paul.
>>
>>> On Apr 19, 2018, at 3:57 PM, Rukmannagari, Shravya <shravya.rukmannagari at intel.com> wrote:
>>>
>>> Hi All,
>>>
>>>
>>>
>>> I would like to contribute a patch  adding vector intrinsics support for Vector API variable shift functions for int and long data types.
>>> Could you please review the patch here:
>>> http://cr.openjdk.java.net/~srukmannagar/VectorAPI_shiftVar/webrev/
>>>
>>>
>>>
>>> Regards,
>>>
>>> Shravya.
>>>
>>