[vector] Is Mask inverted in XXXVector.xxxAll(Mask<>)?

[Deshpande, Vivek R]

Hi Vladimir

Yes you are right the tests for masked reductions are missing.
We can add those with our development progress.

Regards,
Vivek

-----Original Message-----
From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com] 
Sent: Friday, March 22, 2019 4:45 PM
To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>; panama-dev at openjdk.java.net
Subject: Re: [vector] Is Mask inverted in XXXVector.xxxAll(Mask<>)?

Another interesting question is: why the tests haven't caught it?

It seems like masked reductions aren't tested at all.

Best regards,
Vladimir Ivanov

On 22/03/2019 12:22, Deshpande, Vivek R wrote:
> Thanks Vladimir
> 
> I will go ahead push the patch for now to resolve the bug.
> As you said, we should think on improving the naming/ Javadoc in coming time.
> 
> Regards,
> Vivek
> 
> -----Original Message-----
> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
> Sent: Thursday, March 21, 2019 5:47 PM
> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>; 
> panama-dev at openjdk.java.net
> Subject: Re: [vector] Is Mask inverted in XXXVector.xxxAll(Mask<>)?
> 
> 
>> Could you please review the patch for the fix:
>> http://cr.openjdk.java.net/~vdeshpande/VectorAPI/8221176/webrev.00/
> 
> Looks good.
> 
>   From API perspective, what do people think about Vector.blend() as it is specified now?
> 
> It looks naturally when you compare it to other masked operations:
> 
>     a.op(m)       == (m_i ? op(a_i)     : a_i)
> 
>     a.op(b, m)    == (m_i ? op(a_i,b_i) : a_i)
> 
>     a.blend(b, m) == (m_i ? b_i         : a_i) // ok
> 
> but it's confusing (at least, to me) when you see it alone:
> 
>     a.blend(b, m);
> 
> I read it as 'blend(a,b,m)' and the first interpretation usually is opposite to the actual behavior.
> 
> Maybe it can be improved by different naming?
> 
> Best regards,
> Vladimir Ivanov
> 
>> -----Original Message-----
>> From: Vladimir Ivanov [mailto:vladimir.x.ivanov at oracle.com]
>> Sent: Wednesday, March 20, 2019 2:48 PM
>> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>; Lev Serebryakov 
>> <lev at serebryakov.spb.ru>; panama-dev at openjdk.java.net
>> Subject: Re: [vector] Is Mask inverted in XXXVector.xxxAll(Mask<>)?
>>
>>
>>> It is selecting values from the broadcasted vector from base scalar 
>>> value(used as starting point) based on the mask, instead of the vector on which we want to reduce.
>>> I think the arguments to blend  passed should be interchanged.
>>
>> Yes, it agrees with my analysis of the problem (added to JDK-8221176).
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>> -----Original Message-----
>>> From: panama-dev [mailto:panama-dev-bounces at openjdk.java.net] On 
>>> Behalf Of Vladimir Ivanov
>>> Sent: Wednesday, March 20, 2019 2:21 PM
>>> To: Lev Serebryakov <lev at serebryakov.spb.ru>; 
>>> panama-dev at openjdk.java.net
>>> Subject: Re: [vector] Is Mask inverted in XXXVector.xxxAll(Mask<>)?
>>>
>>> Thanks for the report, Lev!
>>>
>>> Yes, it's a bug. Filed
>>> https://bugs.openjdk.java.net/browse/JDK-8221176
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 19/03/2019 09:54, Lev Serebryakov wrote:
>>>>
>>>>      This code gives me 100.0f and not expected 10.0f (Preferred 
>>>> Spices in
>>>> 256 on my system, as I don't have AVX-512):
>>>>
>>>> FloatVector.FloatSpecies PFS = FloatVector.preferredSpecies(); 
>>>> FloatVector.Mask m = FloatVector.maskFromValues(PFS,
>>>>       true, false, true, false, true, false, true, false ); 
>>>> FloatVector v = FloatVector.fromArray(PFS, new float[]
>>>>        { 1.0f, 10.0f, 2.0f, 20.0f, 3.0f, 30.0f, 4.0f, 40.0f }, 0 ); 
>>>> float sum = v.addAll(m);
>>>>
>>>>
>>>>      Same problem with `mulAll()`.
>>>>