Hi,
This patch addresses Segher's findings, and also replaces usages of the
deprecated function vec_vcmpgtfp with the equivalent vec_cmpgt.
Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.
Also tested in a Clang environment with no regressions. Is this ok for
trunk?
Thanks!
Bill
2018-10-24 Bill Schmidt <wschm...@linux.ibm.com>
Jinsong Ji <j...@us.ibm.com>
* config/rs6000/emmintrin.h (_mm_sll_epi64): Remove wrong cast.
* config/rs6000/xmmintrin.h (_mm_min_ps): Change m's type to
__vector __bool int. Use vec_cmpgt in preference to deprecated
function vec_vcmpgtfp.
(_mm_max_ps): Likewise.
Index: gcc/config/rs6000/emmintrin.h
===================================================================
--- gcc/config/rs6000/emmintrin.h (revision 265389)
+++ gcc/config/rs6000/emmintrin.h (working copy)
@@ -1765,8 +1765,7 @@ _mm_sll_epi64 (__m128i __A, __m128i __B)
lshift = (__v2du) vec_splat ((__v2du)__B, 0);
shmask = lshift < shmax;
result = vec_vsld ((__v2du) __A, lshift);
- result = (__v2du) vec_sel ((__v2df) shmask, (__v2df) result,
- (__v2df) shmask);
+ result = (__v2du) vec_sel ((__v2df) shmask, (__v2df) result, shmask);
return (__m128i) result;
}
Index: gcc/config/rs6000/xmmintrin.h
===================================================================
--- gcc/config/rs6000/xmmintrin.h (revision 265389)
+++ gcc/config/rs6000/xmmintrin.h (working copy)
@@ -457,7 +457,7 @@ _mm_max_ss (__m128 __A, __m128 __B)
extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__,
__artificial__))
_mm_min_ps (__m128 __A, __m128 __B)
{
- __m128 m = (__m128) vec_vcmpgtfp ((__v4sf) __B, (__v4sf) __A);
+ __vector __bool int m = vec_cmpgt ((__v4sf) __B, (__v4sf) __A);
return vec_sel (__B, __A, m);
}
@@ -464,7 +464,7 @@ _mm_min_ps (__m128 __A, __m128 __B)
extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__,
__artificial__))
_mm_max_ps (__m128 __A, __m128 __B)
{
- __m128 m = (__m128) vec_vcmpgtfp ((__v4sf) __A, (__v4sf) __B);
+ __vector __bool int m = vec_cmpgt ((__v4sf) __A, (__v4sf) __B);
return vec_sel (__B, __A, m);
}
On 10/22/18 6:49 PM, Segher Boessenkool wrote:
> Hi Bill,
>
> On Mon, Oct 22, 2018 at 01:29:15PM -0500, Bill Schmidt wrote:
>> The vec_sel intrinsic is overloaded for multiple types. There are a
>> couple of cases in our intrinsic compatibility headers where the types
>> used don't match any allowable type signature. GCC is able to correctly
>> infer which matching built-in function is meant, but not all compilers
>> can. For compatibility, cast the third parameter correctly in the
>> source code.
>> --- gcc/config/rs6000/emmintrin.h (revision 265389)
>> +++ gcc/config/rs6000/emmintrin.h (working copy)
>> @@ -1766,7 +1766,7 @@ _mm_sll_epi64 (__m128i __A, __m128i __B)
>> shmask = lshift < shmax;
>> result = vec_vsld ((__v2du) __A, lshift);
>> result = (__v2du) vec_sel ((__v2df) shmask, (__v2df) result,
>> - (__v2df) shmask);
>> + (vector unsigned long long) shmask);
> shmask already is a proper type, just delete the cast? (And then fit this
> on one line).
>
>> --- gcc/config/rs6000/xmmintrin.h (revision 265389)
>> +++ gcc/config/rs6000/xmmintrin.h (working copy)
>> @@ -458,7 +458,7 @@ extern __inline __m128 __attribute__((__gnu_inline
>> _mm_min_ps (__m128 __A, __m128 __B)
>> {
>> __m128 m = (__m128) vec_vcmpgtfp ((__v4sf) __B, (__v4sf) __A);
>> - return vec_sel (__B, __A, m);
>> + return vec_sel (__B, __A, (vector unsigned int)m);
> m should not be type __m128, but maybe __v4si? The cast of the vec_vcmpgtfp
> result can go away as well then, maybe.
>
>
> Segher
>
