[rs6000,v2] Fix uses of vec_sel in intrinsic headers

Message ID 463d71a5-fcfa-89a8-cc2a-08422c55f49f@linux.ibm.com
State New
Headers show
Series
  • [rs6000,v2] Fix uses of vec_sel in intrinsic headers
Related show

Commit Message

Bill Schmidt Oct. 24, 2018, 2:25 p.m.
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  <wschmidt@linux.ibm.com>
	    Jinsong Ji <jji@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.




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

>

Comments

Segher Boessenkool Oct. 25, 2018, 5:08 p.m. | #1
On Wed, Oct 24, 2018 at 09:25:25AM -0500, Bill Schmidt wrote:
> 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?


This is okay for trunk.  Thanks!  Do you want backports to 8?


Segher


> 2018-10-24  Bill Schmidt  <wschmidt@linux.ibm.com>

> 	    Jinsong Ji <jji@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.
Bill Schmidt Oct. 25, 2018, 5:29 p.m. | #2
On 10/25/18 12:08 PM, Segher Boessenkool wrote:
> On Wed, Oct 24, 2018 at 09:25:25AM -0500, Bill Schmidt wrote:

>> 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?

> This is okay for trunk.  Thanks!  Do you want backports to 8?


Yes, thanks!  I should have mentioned that for all of these patches.

Bill

>

>

> Segher

>

>

>> 2018-10-24  Bill Schmidt  <wschmidt@linux.ibm.com>

>> 	    Jinsong Ji <jji@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.

Patch

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);
 }