public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, rs6000] Use explicit casts for vec_sel argument 3 in intrinsic headers
@ 2018-10-22 20:17 Bill Schmidt
  2018-10-23  1:35 ` Segher Boessenkool
  0 siblings, 1 reply; 5+ messages in thread
From: Bill Schmidt @ 2018-10-22 20:17 UTC (permalink / raw)
  To: GCC Patches; +Cc: Segher Boessenkool

Hi,

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.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.
Is this okay for trunk?

Thanks,
Bill


2018-10-22  Bill Schmidt  <wschmidt@linux.ibm.com>
	    Jinsong Ji <jji@us.ibm.com>

	* config/rs6000/emmintrin.h (_mm_sll_epi64): Explicitly cast third
	argument of vec_sel to vector unsigned long long.
	* config/rs6000/xmmintrin.h (_mm_min_ps): Explicitly cast third
	argument of vec_sel to vector unsigned int.


Index: gcc/config/rs6000/emmintrin.h
===================================================================
--- 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);
 
   return (__m128i) result;
 }
Index: gcc/config/rs6000/xmmintrin.h
===================================================================
--- 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);
 }
 
 extern __inline __m128 __attribute__((__gnu_inline__, __always_inline__, __artificial__))

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, rs6000] Use explicit casts for vec_sel argument 3 in intrinsic headers
  2018-10-22 20:17 [PATCH, rs6000] Use explicit casts for vec_sel argument 3 in intrinsic headers Bill Schmidt
@ 2018-10-23  1:35 ` Segher Boessenkool
  2018-10-24 15:10   ` [PATCH, rs6000, v2] Fix uses of vec_sel " Bill Schmidt
  0 siblings, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2018-10-23  1:35 UTC (permalink / raw)
  To: Bill Schmidt; +Cc: GCC Patches

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH, rs6000, v2] Fix uses of vec_sel in intrinsic headers
  2018-10-23  1:35 ` Segher Boessenkool
@ 2018-10-24 15:10   ` Bill Schmidt
  2018-10-25 18:41     ` Segher Boessenkool
  0 siblings, 1 reply; 5+ messages in thread
From: Bill Schmidt @ 2018-10-24 15:10 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches

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.


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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, rs6000, v2] Fix uses of vec_sel in intrinsic headers
  2018-10-24 15:10   ` [PATCH, rs6000, v2] Fix uses of vec_sel " Bill Schmidt
@ 2018-10-25 18:41     ` Segher Boessenkool
  2018-10-25 18:57       ` Bill Schmidt
  0 siblings, 1 reply; 5+ messages in thread
From: Segher Boessenkool @ 2018-10-25 18:41 UTC (permalink / raw)
  To: Bill Schmidt; +Cc: GCC Patches

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH, rs6000, v2] Fix uses of vec_sel in intrinsic headers
  2018-10-25 18:41     ` Segher Boessenkool
@ 2018-10-25 18:57       ` Bill Schmidt
  0 siblings, 0 replies; 5+ messages in thread
From: Bill Schmidt @ 2018-10-25 18:57 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: GCC Patches

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-25 17:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-22 20:17 [PATCH, rs6000] Use explicit casts for vec_sel argument 3 in intrinsic headers Bill Schmidt
2018-10-23  1:35 ` Segher Boessenkool
2018-10-24 15:10   ` [PATCH, rs6000, v2] Fix uses of vec_sel " Bill Schmidt
2018-10-25 18:41     ` Segher Boessenkool
2018-10-25 18:57       ` Bill Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).