[PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi,

  This patch is for internal issue1136. It changes insn condition from
TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions.
These instructions all use DI registers and can be invoked with -mpowerpc64
in a 32-bit environment.

  This patch also changes prototypes of related built-ins and target selector
of test cases.

  Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
Is this okay for trunk? Any recommendations? Thanks a lot.


ChangeLog
2022-08-19  Haochen Gui  <guihaoc@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-builtins.def
	(__builtin_vsx_scalar_extract_exp): Set return type to const unsigned
	long long.
	(__builtin_vsx_scalar_extract_sig): Likewise.
	* config/rs6000/vsx.md (xsxexpdp): Change insn condition from
	TARGET_64BIT to TARGET_POWERPC64.
	(xsxsigdp): Likewise.
	(xsiexpdp): Likewise.
	(xsiexpdpf): Likewise.

gcc/testsuite/
	* gcc.target/powerpc/bfp/scalar-extract-exp-0.c: Change effective
	target from lp64 to has_arch_ppc64 and add -mpowerpc64 for 32-bit
	environment.
	* gcc.target/powerpc/bfp/scalar-extract-sig-0.c: Likewise.
	* gcc.target/powerpc/bfp/scalar-insert-exp-0.c: Likewise.
	* gcc.target/powerpc/bfp/scalar-insert-exp-3.c: Likewise.


patch.diff
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index f76f54793d7..4ebfd4704a1 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -2847,10 +2847,10 @@
   pure vsc __builtin_vsx_lxvl (const void *, signed long);
     LXVL lxvl {}

-  const signed long __builtin_vsx_scalar_extract_exp (double);
+  const unsigned long long __builtin_vsx_scalar_extract_exp (double);
     VSEEDP xsxexpdp {}

-  const signed long __builtin_vsx_scalar_extract_sig (double);
+  const unsigned long long __builtin_vsx_scalar_extract_sig (double);
     VSESDP xsxsigdp {}

   const double __builtin_vsx_scalar_insert_exp (unsigned long long, \
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index e226a93bbe5..a01711aa2cb 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -5098,7 +5098,7 @@ (define_insn "xsxexpdp"
   [(set (match_operand:DI 0 "register_operand" "=r")
 	(unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")]
 	 UNSPEC_VSX_SXEXPDP))]
-  "TARGET_P9_VECTOR && TARGET_64BIT"
+  "TARGET_P9_VECTOR && TARGET_POWERPC64"
   "xsxexpdp %0,%x1"
   [(set_attr "type" "integer")])

@@ -5116,7 +5116,7 @@ (define_insn "xsxsigdp"
   [(set (match_operand:DI 0 "register_operand" "=r")
 	(unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")]
 	 UNSPEC_VSX_SXSIG))]
-  "TARGET_P9_VECTOR && TARGET_64BIT"
+  "TARGET_P9_VECTOR && TARGET_POWERPC64"
   "xsxsigdp %0,%x1"
   [(set_attr "type" "integer")])

@@ -5147,7 +5147,7 @@ (define_insn "xsiexpdp"
 	(unspec:DF [(match_operand:DI 1 "register_operand" "r")
 		    (match_operand:DI 2 "register_operand" "r")]
 	 UNSPEC_VSX_SIEXPDP))]
-  "TARGET_P9_VECTOR && TARGET_64BIT"
+  "TARGET_P9_VECTOR && TARGET_POWERPC64"
   "xsiexpdp %x0,%1,%2"
   [(set_attr "type" "fpsimple")])

@@ -5157,7 +5157,7 @@ (define_insn "xsiexpdpf"
 	(unspec:DF [(match_operand:DF 1 "register_operand" "r")
 		    (match_operand:DI 2 "register_operand" "r")]
 	 UNSPEC_VSX_SIEXPDP))]
-  "TARGET_P9_VECTOR && TARGET_64BIT"
+  "TARGET_P9_VECTOR && TARGET_POWERPC64"
   "xsiexpdp %x0,%1,%2"
   [(set_attr "type" "fpsimple")])

diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
index 35bf1b240f3..c9190bc7c6c 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
@@ -1,7 +1,8 @@
 /* { dg-do compile { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */

 /* This test should succeed only on 64-bit configurations.  */
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
index 637080652b7..a391ac8cce3 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
@@ -1,7 +1,8 @@
 /* { dg-do compile { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */

 /* This test should succeed only on 64-bit configurations.  */
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
index d8243258a67..cd35279e2e7 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
@@ -1,7 +1,8 @@
 /* { dg-do compile { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */

 /* This test should succeed only on 64-bit configurations.  */
 #include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
index 3ecbe3318e8..9c9fe9b9c2f 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
@@ -1,7 +1,8 @@
 /* { dg-do compile { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target powerpc_p9vector_ok } */

 /* This test should succeed only on 64-bit configurations.  */
 #include <altivec.h>

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Kewen.Lin]

Hi Haochen,

on 2022/8/19 10:35, HAO CHEN GUI wrote:
> Hi,
> 
>   This patch is for internal issue1136. It changes insn condition from
> TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions.
> These instructions all use DI registers and can be invoked with -mpowerpc64
> in a 32-bit environment.
> 
>   This patch also changes prototypes of related built-ins and target selector
> of test cases.
> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.
> 
> 
> ChangeLog
> 2022-08-19  Haochen Gui  <guihaoc@linux.ibm.com>
> 
> gcc/
> 	* config/rs6000/rs6000-builtins.def
> 	(__builtin_vsx_scalar_extract_exp): Set return type to const unsigned
> 	long long.
> 	(__builtin_vsx_scalar_extract_sig): Likewise.
> 	* config/rs6000/vsx.md (xsxexpdp): Change insn condition from
> 	TARGET_64BIT to TARGET_POWERPC64.
> 	(xsxsigdp): Likewise.
> 	(xsiexpdp): Likewise.
> 	(xsiexpdpf): Likewise.
> 
> gcc/testsuite/
> 	* gcc.target/powerpc/bfp/scalar-extract-exp-0.c: Change effective
> 	target from lp64 to has_arch_ppc64 and add -mpowerpc64 for 32-bit
> 	environment.
> 	* gcc.target/powerpc/bfp/scalar-extract-sig-0.c: Likewise.
> 	* gcc.target/powerpc/bfp/scalar-insert-exp-0.c: Likewise.
> 	* gcc.target/powerpc/bfp/scalar-insert-exp-3.c: Likewise.
> 
> 
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index f76f54793d7..4ebfd4704a1 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -2847,10 +2847,10 @@
>    pure vsc __builtin_vsx_lxvl (const void *, signed long);
>      LXVL lxvl {}
> 
> -  const signed long __builtin_vsx_scalar_extract_exp (double);
> +  const unsigned long long __builtin_vsx_scalar_extract_exp (double);
>      VSEEDP xsxexpdp {}
> 
> -  const signed long __builtin_vsx_scalar_extract_sig (double);
> +  const unsigned long long __builtin_vsx_scalar_extract_sig (double);
>      VSESDP xsxsigdp {}
> 
>    const double __builtin_vsx_scalar_insert_exp (unsigned long long, \
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index e226a93bbe5..a01711aa2cb 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -5098,7 +5098,7 @@ (define_insn "xsxexpdp"
>    [(set (match_operand:DI 0 "register_operand" "=r")
>  	(unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")]
>  	 UNSPEC_VSX_SXEXPDP))]
> -  "TARGET_P9_VECTOR && TARGET_64BIT"
> +  "TARGET_P9_VECTOR && TARGET_POWERPC64"
>    "xsxexpdp %0,%x1"
>    [(set_attr "type" "integer")])
> 
> @@ -5116,7 +5116,7 @@ (define_insn "xsxsigdp"
>    [(set (match_operand:DI 0 "register_operand" "=r")
>  	(unspec:DI [(match_operand:DF 1 "vsx_register_operand" "wa")]
>  	 UNSPEC_VSX_SXSIG))]
> -  "TARGET_P9_VECTOR && TARGET_64BIT"
> +  "TARGET_P9_VECTOR && TARGET_POWERPC64"
>    "xsxsigdp %0,%x1"
>    [(set_attr "type" "integer")])
> 
> @@ -5147,7 +5147,7 @@ (define_insn "xsiexpdp"
>  	(unspec:DF [(match_operand:DI 1 "register_operand" "r")
>  		    (match_operand:DI 2 "register_operand" "r")]
>  	 UNSPEC_VSX_SIEXPDP))]
> -  "TARGET_P9_VECTOR && TARGET_64BIT"
> +  "TARGET_P9_VECTOR && TARGET_POWERPC64"
>    "xsiexpdp %x0,%1,%2"
>    [(set_attr "type" "fpsimple")])
> 
> @@ -5157,7 +5157,7 @@ (define_insn "xsiexpdpf"
>  	(unspec:DF [(match_operand:DF 1 "register_operand" "r")
>  		    (match_operand:DI 2 "register_operand" "r")]
>  	 UNSPEC_VSX_SIEXPDP))]
> -  "TARGET_P9_VECTOR && TARGET_64BIT"
> +  "TARGET_P9_VECTOR && TARGET_POWERPC64"
>    "xsiexpdp %x0,%1,%2"
>    [(set_attr "type" "fpsimple")])
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> index 35bf1b240f3..c9190bc7c6c 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-require-effective-target powerpc_p9vector_ok } */

Maybe we should add one comment here (also the other touched case) or
in the commit log saying why we reorder the dg-require-effective-target
and dg-options, since the reason isn't obvious.  :)

The others looks good to me.  Thanks!

BR,
Kewen

> 
>  /* This test should succeed only on 64-bit configurations.  */
>  #include <altivec.h>
> diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
> index 637080652b7..a391ac8cce3 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-0.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> 
>  /* This test should succeed only on 64-bit configurations.  */
>  #include <altivec.h>
> diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
> index d8243258a67..cd35279e2e7 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-0.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> 
>  /* This test should succeed only on 64-bit configurations.  */
>  #include <altivec.h>
> diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
> index 3ecbe3318e8..9c9fe9b9c2f 100644
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-3.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> 
>  /* This test should succeed only on 64-bit configurations.  */
>  #include <altivec.h>

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi Kewen,

On 19/8/2022 上午 11:01, Kewen.Lin wrote:
> Maybe we should add one comment here (also the other touched case) or
> in the commit log saying why we reorder the dg-require-effective-target
> and dg-options, since the reason isn't obvious.  :)

Sure, I will explain it in commit log. I submitted an internal issue for
this problem too.

Thanks for your review comments.

Gui Haochen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Segher Boessenkool]

Hi!

On Fri, Aug 19, 2022 at 10:35:54AM +0800, HAO CHEN GUI wrote:
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> @@ -1,7 +1,8 @@
>  /* { dg-do compile { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>  /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */

You can add this always.  It is default on 64-bit systems, but it is
simpler to just always add it:
/* { dg-additional-options "-mpowerpc64" } */

Or are there subtargets that will error on this?

> +/* { dg-require-effective-target has_arch_ppc64 } */

This is redundant, the previous line makes this always pass.


Segher

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi Segher,

On 23/8/2022 下午 10:26, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Aug 19, 2022 at 10:35:54AM +0800, HAO CHEN GUI wrote:
>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
>> @@ -1,7 +1,8 @@
>>  /* { dg-do compile { target { powerpc*-*-* } } } */
>> -/* { dg-require-effective-target lp64 } */
>> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>>  /* { dg-options "-mdejagnu-cpu=power9" } */
>> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> 
> You can add this always.  It is default on 64-bit systems, but it is
> simpler to just always add it:
> /* { dg-additional-options "-mpowerpc64" } */
> 
> Or are there subtargets that will error on this?
Yes, AIX fails if TARGET_POWERPC64 is set and TARGET_64BIT is not set.
So I add "-mpowerpc64" for Linux 32-bit environment.

  if (TARGET_POWERPC64 && ! TARGET_64BIT)                               \
    {                                                                   \
      error ("%<-maix64%> required: 64-bit computation with 32-bit addressing not yet supported"); \
    }

Thanks a lot
Gui Haochen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Kewen.Lin]

on 2022/8/24 13:11, HAO CHEN GUI wrote:
> Hi Segher,
> 
> On 23/8/2022 下午 10:26, Segher Boessenkool wrote:
>> Hi!
>>
>> On Fri, Aug 19, 2022 at 10:35:54AM +0800, HAO CHEN GUI wrote:
>>> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
>>> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
>>> @@ -1,7 +1,8 @@
>>>  /* { dg-do compile { target { powerpc*-*-* } } } */
>>> -/* { dg-require-effective-target lp64 } */
>>> -/* { dg-require-effective-target powerpc_p9vector_ok } */
>>>  /* { dg-options "-mdejagnu-cpu=power9" } */
>>> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
>>
>> You can add this always.  It is default on 64-bit systems, but it is
>> simpler to just always add it:
>> /* { dg-additional-options "-mpowerpc64" } */
>>
>> Or are there subtargets that will error on this?
> Yes, AIX fails if TARGET_POWERPC64 is set and TARGET_64BIT is not set.
> So I add "-mpowerpc64" for Linux 32-bit environment.
> 
>   if (TARGET_POWERPC64 && ! TARGET_64BIT)                               \
>     {                                                                   \
>       error ("%<-maix64%> required: 64-bit computation with 32-bit addressing not yet supported"); \
>     }
> 

Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
having that has_arch_ppc64 effective target on aix?

I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
into an UNSUPPORTED then)?

BR,
Kewen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi Kewen,

On 24/8/2022 下午 1:24, Kewen.Lin wrote:
> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
> having that has_arch_ppc64 effective target on aix?
> 
> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
> into an UNSUPPORTED then)?

Good point. I will get an AIX to test it.

Thanks
Gui Haochen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi,

On 24/8/2022 下午 1:24, Kewen.Lin wrote:
> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
> having that has_arch_ppc64 effective target on aix?
> 
> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
> into an UNSUPPORTED then)?

I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
32-bit AIX environment. It works as we expected.

Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
about them. Not sure if it's intention.

In bfp.exp

# Exit immediately if this isn't a PowerPC target or if the target is
# aix or Darwin.
if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
     || [istarget "powerpc*-*-aix*"]
     || [istarget "powerpc*-*-darwin*"]  } then {
  return
}

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Kewen.Lin]

on 2022/8/25 11:37, HAO CHEN GUI wrote:
> Hi,
> 
> On 24/8/2022 下午 1:24, Kewen.Lin wrote:
>> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
>> having that has_arch_ppc64 effective target on aix?
>>
>> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
>> into an UNSUPPORTED then)?
> 
> I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
> 32-bit AIX environment. It works as we expected.

Nice, thanks for your time on testing.

> 
> Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
> about them. Not sure if it's intention.
> 
> In bfp.exp
> 
> # Exit immediately if this isn't a PowerPC target or if the target is
> # aix or Darwin.
> if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
>      || [istarget "powerpc*-*-aix*"]
>      || [istarget "powerpc*-*-darwin*"]  } then {
>   return
> }

I can't find a hint about why we wanted to disable bfp testing on aix, it looks like a overkill to me.

Could you help to further test if all test cases in this small bucket available on aix?

Maybe it can give us some evidences on why it's intentional or not.

Hi David & Segher,

Do you have some insights on this?

BR,
Kewen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[David Edelsohn]

On Thu, Aug 25, 2022 at 1:22 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> on 2022/8/25 11:37, HAO CHEN GUI wrote:
> > Hi,
> >
> > On 24/8/2022 下午 1:24, Kewen.Lin wrote:
> >> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
> >> having that has_arch_ppc64 effective target on aix?
> >>
> >> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
> >> into an UNSUPPORTED then)?
> >
> > I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
> > 32-bit AIX environment. It works as we expected.
>
> Nice, thanks for your time on testing.
>
> >
> > Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
> > about them. Not sure if it's intention.
> >
> > In bfp.exp
> >
> > # Exit immediately if this isn't a PowerPC target or if the target is
> > # aix or Darwin.
> > if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
> >      || [istarget "powerpc*-*-aix*"]
> >      || [istarget "powerpc*-*-darwin*"]  } then {
> >   return
> > }
>
> I can't find a hint about why we wanted to disable bfp testing on aix, it looks like a overkill to me.
>
> Could you help to further test if all test cases in this small bucket available on aix?
>
> Maybe it can give us some evidences on why it's intentional or not.
>
> Hi David & Segher,
>
> Do you have some insights on this?

AIX (and Darwin) are not Linux and not ELF.  There is no support for
BPF.  All of the tests fail, so they are skipped.

Thanks, David

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[HAO CHEN GUI]

Hi David,

On 25/8/2022 下午 10:01, David Edelsohn wrote:
> On Thu, Aug 25, 2022 at 1:22 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>>
>> on 2022/8/25 11:37, HAO CHEN GUI wrote:
>>> Hi,
>>>
>>> On 24/8/2022 下午 1:24, Kewen.Lin wrote:
>>>> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
>>>> having that has_arch_ppc64 effective target on aix?
>>>>
>>>> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
>>>> into an UNSUPPORTED then)?
>>>
>>> I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
>>> 32-bit AIX environment. It works as we expected.
>>
>> Nice, thanks for your time on testing.
>>
>>>
>>> Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
>>> about them. Not sure if it's intention.
>>>
>>> In bfp.exp
>>>
>>> # Exit immediately if this isn't a PowerPC target or if the target is
>>> # aix or Darwin.
>>> if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
>>>      || [istarget "powerpc*-*-aix*"]
>>>      || [istarget "powerpc*-*-darwin*"]  } then {
>>>   return
>>> }
>>
>> I can't find a hint about why we wanted to disable bfp testing on aix, it looks like a overkill to me.
>>
>> Could you help to further test if all test cases in this small bucket available on aix?
>>
>> Maybe it can give us some evidences on why it's intentional or not.
>>
>> Hi David & Segher,
>>
>> Do you have some insights on this?
> 
> AIX (and Darwin) are not Linux and not ELF.  There is no support for
> BPF.  All of the tests fail, so they are skipped.

Thanks so much for your info.

Here are test results on P7 AIX7.1. I tested all scalar-extract-sig-* and scalar-insert-exp-* cases in
"testsuite/powerpc/bfp" fold. All compiling cases pass except those use __ieee128. The runnable cases
fail as expected. p9vector is not supported on P7 servers.

So the __ieee128 blocks Binary floating-point on AIX?

Thanks
Gui Haochen
> 
> Thanks, David

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Kewen.Lin]

on 2022/8/26 10:42, HAO CHEN GUI wrote:
> Hi David,
> 
> On 25/8/2022 下午 10:01, David Edelsohn wrote:
>> On Thu, Aug 25, 2022 at 1:22 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>>>
>>> on 2022/8/25 11:37, HAO CHEN GUI wrote:
>>>> Hi,
>>>>
>>>> On 24/8/2022 下午 1:24, Kewen.Lin wrote:
>>>>> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
>>>>> having that has_arch_ppc64 effective target on aix?
>>>>>
>>>>> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
>>>>> into an UNSUPPORTED then)?
>>>>
>>>> I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
>>>> 32-bit AIX environment. It works as we expected.
>>>
>>> Nice, thanks for your time on testing.
>>>
>>>>
>>>> Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
>>>> about them. Not sure if it's intention.
>>>>
>>>> In bfp.exp
>>>>
>>>> # Exit immediately if this isn't a PowerPC target or if the target is
>>>> # aix or Darwin.
>>>> if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
>>>>      || [istarget "powerpc*-*-aix*"]
>>>>      || [istarget "powerpc*-*-darwin*"]  } then {
>>>>   return
>>>> }
>>>
>>> I can't find a hint about why we wanted to disable bfp testing on aix, it looks like a overkill to me.
>>>
>>> Could you help to further test if all test cases in this small bucket available on aix?
>>>
>>> Maybe it can give us some evidences on why it's intentional or not.
>>>
>>> Hi David & Segher,
>>>
>>> Do you have some insights on this?
>>
>> AIX (and Darwin) are not Linux and not ELF.  There is no support for
>> BPF.  All of the tests fail, so they are skipped.
> 
> Thanks so much for your info.

+1!

But I guessed the name BFP (short for IEEE binary fp IMHO) was misread as BPF (eBPF)? 

> 
> Here are test results on P7 AIX7.1. I tested all scalar-extract-sig-* and scalar-insert-exp-* cases in
> "testsuite/powerpc/bfp" fold. All compiling cases pass except those use __ieee128. The runnable cases
> fail as expected. p9vector is not supported on P7 servers.

Thanks for testing again.

> 
> So the __ieee128 blocks Binary floating-point on AIX?

We can add one extra effective target ppc_ieee128_ok for them, aix is already excluded in the check.

BR,
Kewen

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[David Edelsohn]

On Thu, Aug 25, 2022 at 10:42 PM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>
> Hi David,
>
> On 25/8/2022 下午 10:01, David Edelsohn wrote:
> > On Thu, Aug 25, 2022 at 1:22 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
> >>
> >> on 2022/8/25 11:37, HAO CHEN GUI wrote:
> >>> Hi,
> >>>
> >>> On 24/8/2022 下午 1:24, Kewen.Lin wrote:
> >>>> Could you try to test with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" all the time, but still
> >>>> having that has_arch_ppc64 effective target on aix?
> >>>>
> >>>> I'd expect has_arch_ppc64 check to fail on aix 32bit, the error will not be a problem (turning
> >>>> into an UNSUPPORTED then)?
> >>>
> >>> I tested it on AIX. "has_arch_ppc64" fails with dg-options "-mdejagnu-cpu=power9 -mpowerpc64" on
> >>> 32-bit AIX environment. It works as we expected.
> >>
> >> Nice, thanks for your time on testing.
> >>
> >>>
> >>> Also I found that AIX and Darwin are skipped for bfp test. So in testcase, it's no need to care
> >>> about them. Not sure if it's intention.
> >>>
> >>> In bfp.exp
> >>>
> >>> # Exit immediately if this isn't a PowerPC target or if the target is
> >>> # aix or Darwin.
> >>> if { (![istarget powerpc*-*-*] && ![istarget rs6000-*-*])
> >>>      || [istarget "powerpc*-*-aix*"]
> >>>      || [istarget "powerpc*-*-darwin*"]  } then {
> >>>   return
> >>> }
> >>
> >> I can't find a hint about why we wanted to disable bfp testing on aix, it looks like a overkill to me.
> >>
> >> Could you help to further test if all test cases in this small bucket available on aix?
> >>
> >> Maybe it can give us some evidences on why it's intentional or not.
> >>
> >> Hi David & Segher,
> >>
> >> Do you have some insights on this?
> >
> > AIX (and Darwin) are not Linux and not ELF.  There is no support for
> > BPF.  All of the tests fail, so they are skipped.
>
> Thanks so much for your info.
>
> Here are test results on P7 AIX7.1. I tested all scalar-extract-sig-* and scalar-insert-exp-* cases in
> "testsuite/powerpc/bfp" fold. All compiling cases pass except those use __ieee128. The runnable cases
> fail as expected. p9vector is not supported on P7 servers.
>
> So the __ieee128 blocks Binary floating-point on AIX?

AIX does not support IEEE 128 bit  floating point, only the IBM
double-double format.  Also GCC for AIX does not recognize the
attributes and options for other formats, although there are some
patches from Mike to address that.

Thanks, David

Re: [PATCH, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions

[Segher Boessenkool]

On Wed, Aug 24, 2022 at 01:11:39PM +0800, HAO CHEN GUI wrote:
> On 23/8/2022 下午 10:26, Segher Boessenkool wrote:
> > On Fri, Aug 19, 2022 at 10:35:54AM +0800, HAO CHEN GUI wrote:
> >> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> >> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-0.c
> >> @@ -1,7 +1,8 @@
> >>  /* { dg-do compile { target { powerpc*-*-* } } } */
> >> -/* { dg-require-effective-target lp64 } */
> >> -/* { dg-require-effective-target powerpc_p9vector_ok } */
> >>  /* { dg-options "-mdejagnu-cpu=power9" } */
> >> +/* { dg-additional-options "-mpowerpc64" { target { powerpc*-*-linux* && ilp32 } } } */
> > 
> > You can add this always.  It is default on 64-bit systems, but it is
> > simpler to just always add it:
> > /* { dg-additional-options "-mpowerpc64" } */
> > 
> > Or are there subtargets that will error on this?
> Yes, AIX fails if TARGET_POWERPC64 is set and TARGET_64BIT is not set.
> So I add "-mpowerpc64" for Linux 32-bit environment.

Aha.  But you can add it for all linux:

/* { dg-additional-options "-mpowerpc64" { target powerpc*-*-linux* } } */

(or *-*-linux* even, everything in gcc.target/powerpc is known to be
powerpc*-*-* already).

Not that it matters at all here, as the other thread shows :-), but for
the future: run testcases wherever possible (and reasonable), and in the
same vein, try not to specialise option when you do not have to.  Doing
this makes testing much less work, makes it easier to have reasonable
coverage.


Segher