This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] (PR84302) Fix _mm_slli_epi{32,64} for shift values 16 through 31 and negative
On 04/13/2018 02:37 PM, Segher Boessenkool wrote:
> On Thu, Apr 12, 2018 at 07:07:21PM -0500, Paul Clarke wrote:
>> The powerpc versions of _mm_slli_epi32 and __mm_slli_epi64 in emmintrin.h
>> do not properly handle shift values between 16 and 31, inclusive.
>> These were setting up the shift with vec_splat_s32, which only accepts
>> *5 bit signed* shift values, or a range of -16 to 15. Values above 15
>> produced an error:
>>
>> error: argument 1 must be a 5-bit signed literal
>>
>> Fix is to effectively reduce the range for which vec_splat_s32 is used
>> to < 32 and use vec_splats otherwise.
>>
>> Also, __mm_slli_epi{16,32,64}, when given a negative shift value,
>> should always return a vector of {0}.
>
> Yup.
>
>> 2018-04-12 Paul A. Clarke <pc@us.ibm.com>
>>
>> gcc/config
>
> There is no changelog there; the changelog is in gcc/. I'll fix it up.
At some point, I'll need to figure out what that means, I guess. :-)
>> --- gcc/config/rs6000/emmintrin.h (revision 259016)
>> +++ gcc/config/rs6000/emmintrin.h (working copy)
>> @@ -1488,7 +1488,7 @@ _mm_slli_epi16 (__m128i __A, int __B)
>> __v8hu lshift;
>> __v8hi result = { 0, 0, 0, 0, 0, 0, 0, 0 };
>>
>> - if (__B < 16)
>> + if (__B > 0 && __B < 16)
>> {
>> if (__builtin_constant_p(__B))
>> lshift = (__v8hu) vec_splat_s16(__B);
>> @@ -1507,12 +1507,12 @@ _mm_slli_epi32 (__m128i __A, int __B)
>> __v4su lshift;
>> __v4si result = { 0, 0, 0, 0 };
>>
>> - if (__B < 32)
>> + if (__B > 0 && __B < 32)
>
> These should >= 0 (I'll fix it).
Oh, you're right, of course. That also means the test cases have to change. Shall I submit a new patch with those changes?
> Thanks for the patch! I'll commit it; please get your commit access in
> order if you plan any further patches.
I'll work on that.
PC