* [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
@ 2022-09-02 8:31 HAO CHEN GUI
2022-09-02 15:56 ` Segher Boessenkool
0 siblings, 1 reply; 11+ messages in thread
From: HAO CHEN GUI @ 2022-09-02 8:31 UTC (permalink / raw)
To: gcc-patches; +Cc: Segher Boessenkool, David, Kewen.Lin, Peter Bergner
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 effective
target 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-09-01 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-exp-6.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-exp-7.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-0.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-6.c: Likewise.
* gcc.target/powerpc/bfp/scalar-extract-sig-7.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-0.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-12.c: Likewise.
* gcc.target/powerpc/bfp/scalar-insert-exp-13.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..81565c50ec7 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-do compile { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" } */
+/* { 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-exp-6.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-6.c
index b9dd7d61aae..33e55d5abc1 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-6.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-6.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* This test should succeed only on 64-bit configurations. */
#include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-7.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-7.c
index a36eae049ec..43c38bb31e4 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-7.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* 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..eb341f1597a 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-do compile { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" } */
+/* { 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-6.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-6.c
index c85072da138..25c0f03142d 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-6.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-6.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* This test should succeed only on 64-bit configurations. */
#include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-7.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-7.c
index 8c94e642c57..43974c08dcb 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-7.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* 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..906d3a07b7d 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-do compile { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" } */
+/* { 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-12.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-12.c
index 384fc9cc675..f027d64632c 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-12.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-12.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* This test should succeed only on 64-bit configurations. */
#include <altivec.h>
diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-13.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-13.c
index 0e004224277..4d3808d8450 100644
--- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-13.c
+++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-13.c
@@ -1,7 +1,7 @@
-/* { dg-do run { target { powerpc*-*-* } } } */
-/* { dg-require-effective-target lp64 } */
-/* { dg-require-effective-target p9vector_hw } */
+/* { dg-do run { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-require-effective-target has_arch_ppc64 } */
+/* { dg-require-effective-target p9vector_hw } */
/* 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..40128470696 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-do compile { target { powerpc*-*-linux* } } } */
/* { dg-options "-mdejagnu-cpu=power9" } */
+/* { dg-additional-options "-mpowerpc64" } */
+/* { 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>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-02 8:31 [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions HAO CHEN GUI
@ 2022-09-02 15:56 ` Segher Boessenkool
2022-09-05 6:36 ` HAO CHEN GUI
0 siblings, 1 reply; 11+ messages in thread
From: Segher Boessenkool @ 2022-09-02 15:56 UTC (permalink / raw)
To: HAO CHEN GUI; +Cc: gcc-patches, David, Kewen.Lin, Peter Bergner
Hi!
On Fri, Sep 02, 2022 at 04:31:38PM +0800, HAO CHEN GUI wrote:
> This patch is for internal issue1136.
This isn't useful to most people. Either just don't mention it here,
or make a public PR for it if that is useful?
> 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.
> gcc/
> * 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-exp-6.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-extract-exp-7.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-extract-sig-0.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-extract-sig-6.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-extract-sig-7.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-insert-exp-0.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-insert-exp-12.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-insert-exp-13.c: Likewise.
> * gcc.target/powerpc/bfp/scalar-insert-exp-3.c: Likewise.
> - 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 {}
This also brings these legacy builtins in line with the vec_ versions,
which are the preferred builtins (they are defined in the PVIPR).
> --- 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")])
This doesn't need POWERPC64 even -- instead, it could use :GPR instead
of :DI, the output is always tiny.
> --- 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-do compile { target { powerpc*-*-linux* } } } */
Why?
> /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-additional-options "-mpowerpc64" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
This is guaranteed already by that -mpowerpc64.
It probably is best if you do not add -mpowerpc64 at all. That solves
both problems, is simpler, and gives better coverage as well :-)
So just use has_arch_ppc64 instead of lp64. That makes it run on a
strict superset of cases :-)
> --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-6.c
> +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-6.c
> @@ -1,7 +1,7 @@
> -/* { dg-do run { target { powerpc*-*-* } } } */
> -/* { dg-require-effective-target lp64 } */
> -/* { dg-require-effective-target p9vector_hw } */
> +/* { dg-do run { target { powerpc*-*-linux* } } } */
> /* { dg-options "-mdejagnu-cpu=power9" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +/* { dg-require-effective-target p9vector_hw } */
Nothing in gcc.target/powerpc has to check for powerpc*-*-* at all. If
you want to test for linux (you shouldn't here afaics?), that is just
*-*-linux* .
Segher
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-02 15:56 ` Segher Boessenkool
@ 2022-09-05 6:36 ` HAO CHEN GUI
2022-09-06 17:19 ` Segher Boessenkool
0 siblings, 1 reply; 11+ messages in thread
From: HAO CHEN GUI @ 2022-09-05 6:36 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches, David, Kewen.Lin, Peter Bergner
Hi Segher,
Thanks for your review comments.
On 2/9/2022 下午 11:56, Segher Boessenkool wrote:
>> - 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 {}
> This also brings these legacy builtins in line with the vec_ versions,
> which are the preferred builtins (they are defined in the PVIPR).
The return type of vec_ version built-ins are different than their definition
in PVIPR. In PVIPR, they're vector unsigned int or vector unsigned long long.
Shall we correct them?
const vd __builtin_vsx_extract_exp_dp (vd);
VEEDP xvxexpdp {}
const vf __builtin_vsx_extract_exp_sp (vf);
VEESP xvxexpsp {}
const vd __builtin_vsx_extract_sig_dp (vd);
VESDP xvxsigdp {}
const vf __builtin_vsx_extract_sig_sp (vf);
VESSP xvxsigsp {}
>
>> --- 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-do compile { target { powerpc*-*-linux* } } } */
> Why?
The powerpc*-*-linux* is no need as bfp.exp excludes AIX and Darwin.
I will modify it.
>
>> /* { dg-options "-mdejagnu-cpu=power9" } */
>> +/* { dg-additional-options "-mpowerpc64" } */
>> +/* { dg-require-effective-target has_arch_ppc64 } */
> This is guaranteed already by that -mpowerpc64.
>
> It probably is best if you do not add -mpowerpc64 at all. That solves
> both problems, is simpler, and gives better coverage as well :-)
>
> So just use has_arch_ppc64 instead of lp64. That makes it run on a
> strict superset of cases :-)
We commonly do regression test with -m32 and -m64. So if -mpowerpc64 is
not added, the combination of "-m32/-mpowerpc64" is not tested.
Thanks
Gui Haochen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-05 6:36 ` HAO CHEN GUI
@ 2022-09-06 17:19 ` Segher Boessenkool
2022-09-07 13:51 ` Paul A. Clarke
2022-09-13 2:34 ` HAO CHEN GUI
0 siblings, 2 replies; 11+ messages in thread
From: Segher Boessenkool @ 2022-09-06 17:19 UTC (permalink / raw)
To: HAO CHEN GUI; +Cc: gcc-patches, David, Kewen.Lin, Peter Bergner
Hi!
On Mon, Sep 05, 2022 at 02:36:30PM +0800, HAO CHEN GUI wrote:
> On 2/9/2022 下午 11:56, Segher Boessenkool wrote:
> >> - 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 {}
> > This also brings these legacy builtins in line with the vec_ versions,
> > which are the preferred builtins (they are defined in the PVIPR).
>
> The return type of vec_ version built-ins are different than their definition
> in PVIPR. In PVIPR, they're vector unsigned int or vector unsigned long long.
> Shall we correct them?
>
> const vd __builtin_vsx_extract_exp_dp (vd);
> VEEDP xvxexpdp {}
>
> const vf __builtin_vsx_extract_exp_sp (vf);
> VEESP xvxexpsp {}
>
> const vd __builtin_vsx_extract_sig_dp (vd);
> VESDP xvxsigdp {}
>
> const vf __builtin_vsx_extract_sig_sp (vf);
> VESSP xvxsigsp {}
Those are the vsx_ versions. I'm not sure what you're asking.
It won't be easy at all to change types from vector integer to vector
float, it will break all over. A compatibility nightmare. It is better
if you can show the current stuff cannot ever work, it's not a problem
to replace it in that case.
> >> --- 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-do compile { target { powerpc*-*-linux* } } } */
> > Why?
> The powerpc*-*-linux* is no need as bfp.exp excludes AIX and Darwin.
> I will modify it.
And powerpc*-*-* is guaranteed in all of gcc.target/powerpc/, so you
need no target clause at all here.
> >> /* { dg-options "-mdejagnu-cpu=power9" } */
> >> +/* { dg-additional-options "-mpowerpc64" } */
> >> +/* { dg-require-effective-target has_arch_ppc64 } */
> > This is guaranteed already by that -mpowerpc64.
> >
> > It probably is best if you do not add -mpowerpc64 at all. That solves
> > both problems, is simpler, and gives better coverage as well :-)
> >
> > So just use has_arch_ppc64 instead of lp64. That makes it run on a
> > strict superset of cases :-)
> We commonly do regression test with -m32 and -m64. So if -mpowerpc64 is
> not added, the combination of "-m32/-mpowerpc64" is not tested.
make -k -j60 check RUNTESTFLAGS="--target_board=unix'{-m64,-m32,-m32/-mpowerpc64}'"
It is fine to not test -m32/-mpowerpc64 so often, and certaionly not
something I will ask everyone to always do :-)
Segher
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-06 17:19 ` Segher Boessenkool
@ 2022-09-07 13:51 ` Paul A. Clarke
2022-09-07 14:25 ` Segher Boessenkool
2022-09-13 2:34 ` HAO CHEN GUI
1 sibling, 1 reply; 11+ messages in thread
From: Paul A. Clarke @ 2022-09-07 13:51 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: HAO CHEN GUI, Peter Bergner, gcc-patches, David
On Tue, Sep 06, 2022 at 12:19:06PM -0500, Segher Boessenkool wrote:
> On Mon, Sep 05, 2022 at 02:36:30PM +0800, HAO CHEN GUI wrote:
> > On 2/9/2022 下午 11:56, Segher Boessenkool wrote:
> > >> - 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 {}
> > > This also brings these legacy builtins in line with the vec_ versions,
> > > which are the preferred builtins (they are defined in the PVIPR).
> >
> > The return type of vec_ version built-ins are different than their definition
> > in PVIPR. In PVIPR, they're vector unsigned int or vector unsigned long long.
> > Shall we correct them?
> >
> > const vd __builtin_vsx_extract_exp_dp (vd);
> > VEEDP xvxexpdp {}
> >
> > const vf __builtin_vsx_extract_exp_sp (vf);
> > VEESP xvxexpsp {}
> >
> > const vd __builtin_vsx_extract_sig_dp (vd);
> > VESDP xvxsigdp {}
> >
> > const vf __builtin_vsx_extract_sig_sp (vf);
> > VESSP xvxsigsp {}
>
> Those are the vsx_ versions. I'm not sure what you're asking.
>
> It won't be easy at all to change types from vector integer to vector
> float, it will break all over. A compatibility nightmare. It is better
> if you can show the current stuff cannot ever work, it's not a problem
> to replace it in that case.
I think Hao Chen is concerned about the return types:
> > const vd __builtin_vsx_extract_exp_dp (vd);
> > VEEDP xvxexpdp {}
Per PVIPR, this should return vector unsigned long long ("vull" not "vd").
> > const vf __builtin_vsx_extract_exp_sp (vf);
> > VEESP xvxexpsp {}
This should return vector unsigned int ("vui" not "vf").
> > const vd __builtin_vsx_extract_sig_dp (vd);
> > VESDP xvxsigdp {}
This should return vector unsigned long long ("vull" not "vd").
> > const vf __builtin_vsx_extract_sig_sp (vf);
> > VESSP xvxsigsp {}
This should return vector unsigned int ("vui" not "vf").
PC
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-07 13:51 ` Paul A. Clarke
@ 2022-09-07 14:25 ` Segher Boessenkool
2022-09-08 5:59 ` HAO CHEN GUI
0 siblings, 1 reply; 11+ messages in thread
From: Segher Boessenkool @ 2022-09-07 14:25 UTC (permalink / raw)
To: Paul A. Clarke; +Cc: HAO CHEN GUI, Peter Bergner, gcc-patches, David
Hi!
On Wed, Sep 07, 2022 at 08:51:17AM -0500, Paul A. Clarke wrote:
> On Tue, Sep 06, 2022 at 12:19:06PM -0500, Segher Boessenkool wrote:
> > On Mon, Sep 05, 2022 at 02:36:30PM +0800, HAO CHEN GUI wrote:
> > > The return type of vec_ version built-ins are different than their definition
> > > in PVIPR. In PVIPR, they're vector unsigned int or vector unsigned long long.
> > > Shall we correct them?
> > >
> > > const vd __builtin_vsx_extract_exp_dp (vd);
> > > VEEDP xvxexpdp {}
> > >
> > > const vf __builtin_vsx_extract_exp_sp (vf);
> > > VEESP xvxexpsp {}
> > >
> > > const vd __builtin_vsx_extract_sig_dp (vd);
> > > VESDP xvxsigdp {}
> > >
> > > const vf __builtin_vsx_extract_sig_sp (vf);
> > > VESSP xvxsigsp {}
> >
> > Those are the vsx_ versions. I'm not sure what you're asking.
> >
> > It won't be easy at all to change types from vector integer to vector
> > float, it will break all over. A compatibility nightmare. It is better
> > if you can show the current stuff cannot ever work, it's not a problem
> > to replace it in that case.
>
> I think Hao Chen is concerned about the return types:
Yes, and so am I.
> > > const vd __builtin_vsx_extract_exp_dp (vd);
> > > VEEDP xvxexpdp {}
>
> Per PVIPR, this should return vector unsigned long long ("vull" not "vd").
But changing that will make any existing code that now works, fail
horribly. Of course it is possible no such code exists :-)
What did this do before the builtin rewrite?
~ - ~ - ~
It looks like it did the right thing before, but that is just based on
reading the code, I haven't actually tried it :-)
So, changing the vsx_ code here should be okay, because obviously no one
is using it. OTOH, why do we have those separately at all, why do they
not just redirect to the canonical vec_ versions? Or, can we just get
rid of the vsx_ version completely?
Segher
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-07 14:25 ` Segher Boessenkool
@ 2022-09-08 5:59 ` HAO CHEN GUI
2022-09-09 17:17 ` Segher Boessenkool
0 siblings, 1 reply; 11+ messages in thread
From: HAO CHEN GUI @ 2022-09-08 5:59 UTC (permalink / raw)
To: Segher Boessenkool, Paul A. Clarke; +Cc: Peter Bergner, gcc-patches, David
On 7/9/2022 下午 10:25, Segher Boessenkool wrote:
> Hi!
>
> On Wed, Sep 07, 2022 at 08:51:17AM -0500, Paul A. Clarke wrote:
>> On Tue, Sep 06, 2022 at 12:19:06PM -0500, Segher Boessenkool wrote:
>>> On Mon, Sep 05, 2022 at 02:36:30PM +0800, HAO CHEN GUI wrote:
>>>> The return type of vec_ version built-ins are different than their definition
>>>> in PVIPR. In PVIPR, they're vector unsigned int or vector unsigned long long.
>>>> Shall we correct them?
>>>>
>>>> const vd __builtin_vsx_extract_exp_dp (vd);
>>>> VEEDP xvxexpdp {}
>>>>
>>>> const vf __builtin_vsx_extract_exp_sp (vf);
>>>> VEESP xvxexpsp {}
>>>>
>>>> const vd __builtin_vsx_extract_sig_dp (vd);
>>>> VESDP xvxsigdp {}
>>>>
>>>> const vf __builtin_vsx_extract_sig_sp (vf);
>>>> VESSP xvxsigsp {}
>>>
>>> Those are the vsx_ versions. I'm not sure what you're asking.
>>>
>>> It won't be easy at all to change types from vector integer to vector
>>> float, it will break all over. A compatibility nightmare. It is better
>>> if you can show the current stuff cannot ever work, it's not a problem
>>> to replace it in that case.
>>
>> I think Hao Chen is concerned about the return types:
>
> Yes, and so am I.
>
>>>> const vd __builtin_vsx_extract_exp_dp (vd);
>>>> VEEDP xvxexpdp {}
>>
>> Per PVIPR, this should return vector unsigned long long ("vull" not "vd").
>
> But changing that will make any existing code that now works, fail
> horribly. Of course it is possible no such code exists :-)
>
> What did this do before the builtin rewrite?
>
>
> ~ - ~ - ~
>
>
> It looks like it did the right thing before, but that is just based on
> reading the code, I haven't actually tried it :-)
>
> So, changing the vsx_ code here should be okay, because obviously no one
> is using it. OTOH, why do we have those separately at all, why do they
> not just redirect to the canonical vec_ versions? Or, can we just get
> rid of the vsx_ version completely?
In rs6000-overload.def, the vsx_ version built-ins are overridden to vec_
version. And the return types of vec_ version is inline with those defined
in PVIPR. So there should be no problem. Sorry for that.
[VEC_VEEDP, vec_extract_exp_dp, __builtin_vec_extract_exp_dp]
vull __builtin_vec_extract_exp_dp (vd);
VEEDP VEEDP_DEPR1
[VEC_VEESP, vec_extract_exp_sp, __builtin_vec_extract_exp_sp]
vui __builtin_vec_extract_exp_sp (vf);
VEESP VEESP_DEPR1
[VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
vui __builtin_vec_extract_exp (vf);
VEESP
vull __builtin_vec_extract_exp (vd);
VEEDP
Thanks
Gui Haochen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-08 5:59 ` HAO CHEN GUI
@ 2022-09-09 17:17 ` Segher Boessenkool
2022-09-13 2:13 ` HAO CHEN GUI
0 siblings, 1 reply; 11+ messages in thread
From: Segher Boessenkool @ 2022-09-09 17:17 UTC (permalink / raw)
To: HAO CHEN GUI; +Cc: Paul A. Clarke, Peter Bergner, gcc-patches, David
On Thu, Sep 08, 2022 at 01:59:02PM +0800, HAO CHEN GUI wrote:
> In rs6000-overload.def, the vsx_ version built-ins are overridden to vec_
> version.
How? Where?
Instead, afaics they are defined in rs6000-builtins.def:
const vd __builtin_vsx_extract_exp_dp (vd);
VEEDP xvxexpdp {}
const vf __builtin_vsx_extract_exp_sp (vf);
VEESP xvxexpsp {}
const vd __builtin_vsx_extract_sig_dp (vd);
VESDP xvxsigdp {}
const vf __builtin_vsx_extract_sig_sp (vf);
VESSP xvxsigsp {}
Again: the vec_ versions are fine. I wonder if the vsx_ versions ever
worked, if the builtin infrastructure rewrite broke it. And if so, what
we should do now? The argument for not deleting these legacy builtins
is that someone might use them, but that seems unlikely since it has
been utterly broken for a while now, and no one has complained.
Segher
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-09 17:17 ` Segher Boessenkool
@ 2022-09-13 2:13 ` HAO CHEN GUI
0 siblings, 0 replies; 11+ messages in thread
From: HAO CHEN GUI @ 2022-09-13 2:13 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Paul A. Clarke, Peter Bergner, gcc-patches, David
Hi Segher,
On 10/9/2022 上午 1:17, Segher Boessenkool wrote:
>> In rs6000-overload.def, the vsx_ version built-ins are overridden to vec_
>> version.
> How? Where?
vec_ version built-ins are defined in rs6000-overload.def. Yes, they're fine
and in line with the definition in PVIPR.
[VEC_VEEDP, vec_extract_exp_dp, __builtin_vec_extract_exp_dp]
vull __builtin_vec_extract_exp_dp (vd);
VEEDP VEEDP_DEPR1
[VEC_VEESP, vec_extract_exp_sp, __builtin_vec_extract_exp_sp]
vui __builtin_vec_extract_exp_sp (vf);
VEESP VEESP_DEPR1
[VEC_VEE, vec_extract_exp, __builtin_vec_extract_exp]
vui __builtin_vec_extract_exp (vf);
VEESP
vull __builtin_vec_extract_exp (vd);
VEEDP
vec-extract-exp-2.c:12:3: note: overloaded builtin ‘__builtin_vec_extract_exp’ is implemented by builtin ‘__builtin_vsx_extract_exp_dp’
Thanks
Gui Haochen
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-06 17:19 ` Segher Boessenkool
2022-09-07 13:51 ` Paul A. Clarke
@ 2022-09-13 2:34 ` HAO CHEN GUI
2022-09-13 7:08 ` Iain Sandoe
1 sibling, 1 reply; 11+ messages in thread
From: HAO CHEN GUI @ 2022-09-13 2:34 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: gcc-patches, David, Kewen.Lin, Peter Bergner
Hi Segher,
On 7/9/2022 上午 1:19, Segher Boessenkool wrote:
> make -k -j60 check RUNTESTFLAGS="--target_board=unix'{-m64,-m32,-m32/-mpowerpc64}'"
>
> It is fine to not test -m32/-mpowerpc64 so often, and certaionly not
> something I will ask everyone to always do :-)
IMO, if we add "-mpowerpc64" into dg-options, the "-m32/-mpowerpc64" will be tested
automatically. It will increase the test coverage. So the concern is it increases test
time?
Thanks
Gui Haochen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions
2022-09-13 2:34 ` HAO CHEN GUI
@ 2022-09-13 7:08 ` Iain Sandoe
0 siblings, 0 replies; 11+ messages in thread
From: Iain Sandoe @ 2022-09-13 7:08 UTC (permalink / raw)
To: HAO CHEN GUI
Cc: Segher Boessenkool, Peter Bergner, GCC Patches, David Edelsohn
Hi
> On 13 Sep 2022, at 03:34, HAO CHEN GUI via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On 7/9/2022 上午 1:19, Segher Boessenkool wrote:
>> make -k -j60 check RUNTESTFLAGS="--target_board=unix'{-m64,-m32,-m32/-mpowerpc64}'"
>>
>> It is fine to not test -m32/-mpowerpc64 so often, and certaionly not
>> something I will ask everyone to always do :-)
>
> IMO, if we add "-mpowerpc64" into dg-options, the "-m32/-mpowerpc64" will be tested
> automatically. It will increase the test coverage. So the concern is it increases test
> time?
Do you mean for the powerpc-target tests, or for the entire suite?
- 50% increase is pretty significant to the entire suite. It would probably only be acceptable on
the newer powerful machines.
The stategy would need to be a little more subtle than adding it everywhere - it is not applicable
to 32b targets running on 32b hardware - however, it *is* potentially applicable to 32b targets
running on 64b hardware (I have one of those).
====
There are tricks you can play with dejagnu to automate this for your setup… by pointing the
DEJAGNU environment to a script (or by using .dejagnurc in your home directory).
You can add customisation in this way, even to set different parameters per host. This saves
me from remembering to put the RUNTESTFLAGS=… extras for the target on each test line.
— so here’s an example as I use it..
I have an environment/script setup like this (which I use to control what is done across a range of
machines with capabilities that vary by at least a factor of 10):
DEJAGNU=/path/to/gcc-runtest-site.exp
gcc-runtest-site.exp:
global target_list
global tool_timeout
case "$target_triplet" in {
{ "*-*-darwin[5-8]*" } {
set target_list { "unix" }
set tool_timeout 240
puts "tool timeout 3mins"
}
{ "*-*-darwin9*" } {
set target_list { "unix{-m32,-m64}" }
set tool_timeout 240
}
{ "i?86-*-darwin10*" } {
set target_list { "unix" }
}
{ "powerpc-*-darwin10*" } {
set target_list { "unix" }
}
{ "*-*-darwin1[01234567]*" } {
set target_list { "unix{-m64,-m32}" }
set tool_timeout 120
puts "gcc-runtest-site : tool timeout 2 mins"
}
{ "*-*-darwin1[89]*" } {
set target_list { "unix" }
set tool_timeout 60
puts "gcc-runtest-site : tool timeout 1 min"
}
{ "x86_64-*-linux-gnu" } {
set target_list { "unix{-m32,-m64}" }
set tool_timeout 60
}
<SNIP - you get the idea, I am sure>
default {
# Works for most unixy things and for darwin < 9.
set target_list { "unix" }
}
}
- sometimes I override this with specific values (especially for timeout) in ~/..dejagnurc on
very slow boxes.
====
0.02GBP only,
cheers
Iain
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-09-13 7:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-02 8:31 [PATCH v2, rs6000] Change insn condition from TARGET_64BIT to TARGET_POWERPC64 for VSX scalar extract/insert instructions HAO CHEN GUI
2022-09-02 15:56 ` Segher Boessenkool
2022-09-05 6:36 ` HAO CHEN GUI
2022-09-06 17:19 ` Segher Boessenkool
2022-09-07 13:51 ` Paul A. Clarke
2022-09-07 14:25 ` Segher Boessenkool
2022-09-08 5:59 ` HAO CHEN GUI
2022-09-09 17:17 ` Segher Boessenkool
2022-09-13 2:13 ` HAO CHEN GUI
2022-09-13 2:34 ` HAO CHEN GUI
2022-09-13 7:08 ` Iain Sandoe
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).