diff mbox series

PR106342 - IBM zSystems: Provide vsel for all vector modes

Message ID 20220803102039.514517-1-iii@linux.ibm.com
State New, archived
Headers
Series PR106342 - IBM zSystems: Provide vsel for all vector modes |

Commit Message

Ilya Leoshkevich Aug. 3, 2022, 10:20 a.m. UTC 
  Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?



dg.exp=pr104612.c fails with an ICE on s390x, because copysignv2sf3
produces an insn that vsel<mode> is supposed to recognize, but can't,
because it's not defined for V2SF.  Fix by defining it for all vector
modes supported by copysign<mode>3.

gcc/ChangeLog:

	* config/s390/vector.md (V_HW_FT): New iterator.
	* config/s390/vx-builtins.md (vsel<mode>): Use V instead of
	V_HW.
---
 gcc/config/s390/vector.md      |  6 ++++++
 gcc/config/s390/vx-builtins.md | 12 ++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

Comments

Andreas Krebbel Aug. 3, 2022, 1:41 p.m. UTC | #1 
On 8/3/22 12:20, Ilya Leoshkevich wrote:
> Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?
> 
> 
> 
> dg.exp=pr104612.c fails with an ICE on s390x, because copysignv2sf3
> produces an insn that vsel<mode> is supposed to recognize, but can't,
> because it's not defined for V2SF.  Fix by defining it for all vector
> modes supported by copysign<mode>3.
> 
> gcc/ChangeLog:
> 
> 	* config/s390/vector.md (V_HW_FT): New iterator.
> 	* config/s390/vx-builtins.md (vsel<mode>): Use V instead of
> 	V_HW.

Ok. There is a typo in the changelog:
"Use *V* instead ..." should probably read "Use V_HW_FT instead ..."

Thanks,

Andreas

> ---
>  gcc/config/s390/vector.md      |  6 ++++++
>  gcc/config/s390/vx-builtins.md | 12 ++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
> index a6c4b4eb974..624729814af 100644
> --- a/gcc/config/s390/vector.md
> +++ b/gcc/config/s390/vector.md
> @@ -63,6 +63,12 @@
>  			   V1DF V2DF
>  			   (V1TF "TARGET_VXE") (TF "TARGET_VXE")])
>  
> +; All modes present in V_HW and VFT.
> +(define_mode_iterator V_HW_FT [V16QI V8HI V4SI V2DI (V1TI "TARGET_VXE") V1DF
> +			       V2DF (V1SF "TARGET_VXE") (V2SF "TARGET_VXE")
> +			       (V4SF "TARGET_VXE") (V1TF "TARGET_VXE")
> +			       (TF "TARGET_VXE")])
> +
>  ; FP vector modes directly supported by the HW.  This does not include
>  ; vector modes using only part of a vector register and should be used
>  ; for instructions which might trigger IEEE exceptions.
> diff --git a/gcc/config/s390/vx-builtins.md b/gcc/config/s390/vx-builtins.md
> index d5130799804..98ee08b2683 100644
> --- a/gcc/config/s390/vx-builtins.md
> +++ b/gcc/config/s390/vx-builtins.md
> @@ -517,12 +517,12 @@
>  ; swapped in s390-c.cc when we get here.
>  
>  (define_insn "vsel<mode>"
> -  [(set (match_operand:V_HW                      0 "register_operand" "=v")
> -	(ior:V_HW
> -	 (and:V_HW (match_operand:V_HW           1 "register_operand"  "v")
> -		   (match_operand:V_HW           3 "register_operand"  "v"))
> -	 (and:V_HW (not:V_HW (match_dup 3))
> -		   (match_operand:V_HW           2 "register_operand"  "v"))))]
> +  [(set (match_operand:V_HW_FT               0 "register_operand" "=v")
> +	(ior:V_HW_FT
> +	 (and:V_HW_FT (match_operand:V_HW_FT 1 "register_operand"  "v")
> +		      (match_operand:V_HW_FT 3 "register_operand"  "v"))
> +	 (and:V_HW_FT (not:V_HW_FT (match_dup 3))
> +		      (match_operand:V_HW_FT 2 "register_operand"  "v"))))]
>    "TARGET_VX"
>    "vsel\t%v0,%1,%2,%3"
>    [(set_attr "op_type" "VRR")])
Ilya Leoshkevich Aug. 10, 2022, 11:42 a.m. UTC | #2 
On Wed, 2022-08-03 at 12:20 +0200, Ilya Leoshkevich wrote:
> Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?
> 
> 
> 
> dg.exp=pr104612.c fails with an ICE on s390x, because copysignv2sf3
> produces an insn that vsel<mode> is supposed to recognize, but can't,
> because it's not defined for V2SF.  Fix by defining it for all vector
> modes supported by copysign<mode>3.
> 
> gcc/ChangeLog:
> 
>         * config/s390/vector.md (V_HW_FT): New iterator.
>         * config/s390/vx-builtins.md (vsel<mode>): Use V instead of
>         V_HW.
> ---
>  gcc/config/s390/vector.md      |  6 ++++++
>  gcc/config/s390/vx-builtins.md | 12 ++++++------
>  2 files changed, 12 insertions(+), 6 deletions(-)

Jakub pointed out that this is broken in gcc-12 as well.
The patch applies cleanly, and I started a bootstrap/regtest.
Ok for gcc-12?
Andreas Krebbel Aug. 11, 2022, 5:45 a.m. UTC | #3 
On 8/10/22 13:42, Ilya Leoshkevich wrote:
> On Wed, 2022-08-03 at 12:20 +0200, Ilya Leoshkevich wrote:
>> Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?
>>
>>
>>
>> dg.exp=pr104612.c fails with an ICE on s390x, because copysignv2sf3
>> produces an insn that vsel<mode> is supposed to recognize, but can't,
>> because it's not defined for V2SF.  Fix by defining it for all vector
>> modes supported by copysign<mode>3.
>>
>> gcc/ChangeLog:
>>
>>         * config/s390/vector.md (V_HW_FT): New iterator.
>>         * config/s390/vx-builtins.md (vsel<mode>): Use V instead of
>>         V_HW.
>> ---
>>  gcc/config/s390/vector.md      |  6 ++++++
>>  gcc/config/s390/vx-builtins.md | 12 ++++++------
>>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> Jakub pointed out that this is broken in gcc-12 as well.
> The patch applies cleanly, and I started a bootstrap/regtest.
> Ok for gcc-12?

Yes. Thanks!

Andreas
Ilya Leoshkevich Aug. 17, 2022, 1:01 p.m. UTC | #4 
On Thu, 2022-08-11 at 07:45 +0200, Andreas Krebbel wrote:
> On 8/10/22 13:42, Ilya Leoshkevich wrote:
> > On Wed, 2022-08-03 at 12:20 +0200, Ilya Leoshkevich wrote:
> > > Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?
> > > 
> > > 
> > > 
> > > dg.exp=pr104612.c fails with an ICE on s390x, because
> > > copysignv2sf3
> > > produces an insn that vsel<mode> is supposed to recognize, but
> > > can't,
> > > because it's not defined for V2SF.  Fix by defining it for all
> > > vector
> > > modes supported by copysign<mode>3.
> > > 
> > > gcc/ChangeLog:
> > > 
> > >         * config/s390/vector.md (V_HW_FT): New iterator.
> > >         * config/s390/vx-builtins.md (vsel<mode>): Use V instead
> > > of
> > >         V_HW.
> > > ---
> > >  gcc/config/s390/vector.md      |  6 ++++++
> > >  gcc/config/s390/vx-builtins.md | 12 ++++++------
> > >  2 files changed, 12 insertions(+), 6 deletions(-)
> > 
> > Jakub pointed out that this is broken in gcc-12 as well.
> > The patch applies cleanly, and I started a bootstrap/regtest.
> > Ok for gcc-12?
> 
> Yes. Thanks!
> 
> Andreas

Hi,

I've committed this today without realizing that gcc-12 branch is
closed.  Sorry!  Please let me know if I should revert this.

Best regards,
Ilya
Richard Biener Aug. 18, 2022, 6:44 a.m. UTC | #5 
On Wed, 17 Aug 2022, Ilya Leoshkevich wrote:

> On Thu, 2022-08-11 at 07:45 +0200, Andreas Krebbel wrote:
> > On 8/10/22 13:42, Ilya Leoshkevich wrote:
> > > On Wed, 2022-08-03 at 12:20 +0200, Ilya Leoshkevich wrote:
> > > > Bootstrapped and regtested on s390x-redhat-linux.  Ok for master?
> > > > 
> > > > 
> > > > 
> > > > dg.exp=pr104612.c fails with an ICE on s390x, because
> > > > copysignv2sf3
> > > > produces an insn that vsel<mode> is supposed to recognize, but
> > > > can't,
> > > > because it's not defined for V2SF.  Fix by defining it for all
> > > > vector
> > > > modes supported by copysign<mode>3.
> > > > 
> > > > gcc/ChangeLog:
> > > > 
> > > >         * config/s390/vector.md (V_HW_FT): New iterator.
> > > >         * config/s390/vx-builtins.md (vsel<mode>): Use V instead
> > > > of
> > > >         V_HW.
> > > > ---
> > > >  gcc/config/s390/vector.md      |  6 ++++++
> > > >  gcc/config/s390/vx-builtins.md | 12 ++++++------
> > > >  2 files changed, 12 insertions(+), 6 deletions(-)
> > > 
> > > Jakub pointed out that this is broken in gcc-12 as well.
> > > The patch applies cleanly, and I started a bootstrap/regtest.
> > > Ok for gcc-12?
> > 
> > Yes. Thanks!
> > 
> > Andreas
> 
> Hi,
> 
> I've committed this today without realizing that gcc-12 branch is
> closed.  Sorry!  Please let me know if I should revert this.

It doesn't look too risky, so leave it in.

Richard.
diff mbox series

Patch

diff --git a/gcc/config/s390/vector.md b/gcc/config/s390/vector.md
index a6c4b4eb974..624729814af 100644
--- a/gcc/config/s390/vector.md
+++ b/gcc/config/s390/vector.md
@@ -63,6 +63,12 @@ 
 			   V1DF V2DF
 			   (V1TF "TARGET_VXE") (TF "TARGET_VXE")])
 
+; All modes present in V_HW and VFT.
+(define_mode_iterator V_HW_FT [V16QI V8HI V4SI V2DI (V1TI "TARGET_VXE") V1DF
+			       V2DF (V1SF "TARGET_VXE") (V2SF "TARGET_VXE")
+			       (V4SF "TARGET_VXE") (V1TF "TARGET_VXE")
+			       (TF "TARGET_VXE")])
+
 ; FP vector modes directly supported by the HW.  This does not include
 ; vector modes using only part of a vector register and should be used
 ; for instructions which might trigger IEEE exceptions.
diff --git a/gcc/config/s390/vx-builtins.md b/gcc/config/s390/vx-builtins.md
index d5130799804..98ee08b2683 100644
--- a/gcc/config/s390/vx-builtins.md
+++ b/gcc/config/s390/vx-builtins.md
@@ -517,12 +517,12 @@ 
 ; swapped in s390-c.cc when we get here.
 
 (define_insn "vsel<mode>"
-  [(set (match_operand:V_HW                      0 "register_operand" "=v")
-	(ior:V_HW
-	 (and:V_HW (match_operand:V_HW           1 "register_operand"  "v")
-		   (match_operand:V_HW           3 "register_operand"  "v"))
-	 (and:V_HW (not:V_HW (match_dup 3))
-		   (match_operand:V_HW           2 "register_operand"  "v"))))]
+  [(set (match_operand:V_HW_FT               0 "register_operand" "=v")
+	(ior:V_HW_FT
+	 (and:V_HW_FT (match_operand:V_HW_FT 1 "register_operand"  "v")
+		      (match_operand:V_HW_FT 3 "register_operand"  "v"))
+	 (and:V_HW_FT (not:V_HW_FT (match_dup 3))
+		      (match_operand:V_HW_FT 2 "register_operand"  "v"))))]
   "TARGET_VX"
   "vsel\t%v0,%1,%2,%3"
   [(set_attr "op_type" "VRR")])