[10/10,RISCV] Fix PR 106632 and PR 106588 a few constraints in bitmanip.md

Message ID 1660860233-11175-11-git-send-email-apinski@marvell.com
State New
Headers
Series Fix/improve the RISCV backend |

Commit Message

Li, Pan2 via Gcc-patches Aug. 18, 2022, 10:03 p.m. UTC
  From: Andrew Pinski <apinski@marvell.com>

The constraints should be n instead of i. Also there
needs to a check for out of bounds zero_extract for
*bexti.

gcc/ChangeLog:

	PR target/106632
	PR target/106588
	* config/riscv/bitmanip.md (*shNadduw): Use n constraint
	instead of i.
	(*slliuw): Likewise.
	(*bexti): Likewise. Also add a check for operands[2] to be less
	than the mode bitsize.
---
 gcc/config/riscv/bitmanip.md | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
  

Comments

Kito Cheng Aug. 22, 2022, 9:09 a.m. UTC | #1
On Fri, Aug 19, 2022 at 6:08 AM apinski--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Andrew Pinski <apinski@marvell.com>
>
> The constraints should be n instead of i. Also there
> needs to a check for out of bounds zero_extract for
> *bexti.
>
> gcc/ChangeLog:
>
>         PR target/106632
>         PR target/106588
>         * config/riscv/bitmanip.md (*shNadduw): Use n constraint
>         instead of i.
>         (*slliuw): Likewise.
>         (*bexti): Likewise. Also add a check for operands[2] to be less
>         than the mode bitsize.
> ---
>  gcc/config/riscv/bitmanip.md | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index 026299d6703..ecf5b51b533 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -44,7 +44,7 @@ (define_insn "*shNadduw"
>         (plus:DI
>           (and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
>                              (match_operand:QI 2 "imm123_operand" "Ds3"))
> -                (match_operand 3 "immediate_operand" ""))
> +                (match_operand 3 "immediate_operand" "n"))
>           (match_operand:DI 4 "register_operand" "r")))]
>    "TARGET_64BIT && TARGET_ZBA
>     && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
> @@ -110,7 +110,7 @@ (define_insn "*slliuw"
>    [(set (match_operand:DI 0 "register_operand" "=r")
>         (and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
>                            (match_operand:QI 2 "immediate_operand" "I"))
> -               (match_operand 3 "immediate_operand" "")))]
> +               (match_operand 3 "immediate_operand" "n")))]
>    "TARGET_64BIT && TARGET_ZBA
>     && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
>    "slli.uw\t%0,%1,%2"
> @@ -354,6 +354,7 @@ (define_insn "*bexti"
>         (zero_extract:X (match_operand:X 1 "register_operand" "r")
>                         (const_int 1)
>                         (match_operand 2 "immediate_operand" "i")))]

Seems something wrong during generating this patch, this line should be removed.

> -  "TARGET_ZBS"
> +                       (match_operand 2 "immediate_operand" "n")))]
> +  "TARGET_ZBS && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
>    "bexti\t%0,%1,%2"
>    [(set_attr "type" "bitmanip")])
> --
> 2.27.0
>
  
Palmer Dabbelt Aug. 22, 2022, 8:44 p.m. UTC | #2
On Thu, 18 Aug 2022 15:03:53 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> From: Andrew Pinski <apinski@marvell.com>
>
> The constraints should be n instead of i. Also there
> needs to a check for out of bounds zero_extract for
> *bexti.
>
> gcc/ChangeLog:
>
> 	PR target/106632
> 	PR target/106588
> 	* config/riscv/bitmanip.md (*shNadduw): Use n constraint
> 	instead of i.
> 	(*slliuw): Likewise.
> 	(*bexti): Likewise. Also add a check for operands[2] to be less
> 	than the mode bitsize.
> ---
>  gcc/config/riscv/bitmanip.md | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
> index 026299d6703..ecf5b51b533 100644
> --- a/gcc/config/riscv/bitmanip.md
> +++ b/gcc/config/riscv/bitmanip.md
> @@ -44,7 +44,7 @@ (define_insn "*shNadduw"
>  	(plus:DI
>  	  (and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
>  			     (match_operand:QI 2 "imm123_operand" "Ds3"))
> -		 (match_operand 3 "immediate_operand" ""))
> +		 (match_operand 3 "immediate_operand" "n"))
>  	  (match_operand:DI 4 "register_operand" "r")))]
>    "TARGET_64BIT && TARGET_ZBA
>     && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
> @@ -110,7 +110,7 @@ (define_insn "*slliuw"
>    [(set (match_operand:DI 0 "register_operand" "=r")
>  	(and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
>  			   (match_operand:QI 2 "immediate_operand" "I"))
> -		(match_operand 3 "immediate_operand" "")))]
> +		(match_operand 3 "immediate_operand" "n")))]
>    "TARGET_64BIT && TARGET_ZBA
>     && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
>    "slli.uw\t%0,%1,%2"
> @@ -354,6 +354,7 @@ (define_insn "*bexti"
>  	(zero_extract:X (match_operand:X 1 "register_operand" "r")
>  			(const_int 1)
>  			(match_operand 2 "immediate_operand" "i")))]
> -  "TARGET_ZBS"
> +			(match_operand 2 "immediate_operand" "n")))]
> +  "TARGET_ZBS && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
>    "bexti\t%0,%1,%2"
>    [(set_attr "type" "bitmanip")])

I think something went off the rails on that last chunk and it should 
look more like

@@ -353,7 +353,7 @@
   [(set (match_operand:X 0 "register_operand" "=r")
        (zero_extract:X (match_operand:X 1 "register_operand" "r")
                        (const_int 1)
-                       (match_operand 2 "immediate_operand" "i")))]
-  "TARGET_ZBS"
+                       (match_operand 2 "immediate_operand" "n")))]
+  "TARGET_ZBS && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
   "bexti\t%0,%1,%2"
   [(set_attr "type" "bitmanip")])

with that I get no new failures on trunk with all of these (though I'm 
still only testing the Linux multilibs for now).
  

Patch

diff --git a/gcc/config/riscv/bitmanip.md b/gcc/config/riscv/bitmanip.md
index 026299d6703..ecf5b51b533 100644
--- a/gcc/config/riscv/bitmanip.md
+++ b/gcc/config/riscv/bitmanip.md
@@ -44,7 +44,7 @@  (define_insn "*shNadduw"
 	(plus:DI
 	  (and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
 			     (match_operand:QI 2 "imm123_operand" "Ds3"))
-		 (match_operand 3 "immediate_operand" ""))
+		 (match_operand 3 "immediate_operand" "n"))
 	  (match_operand:DI 4 "register_operand" "r")))]
   "TARGET_64BIT && TARGET_ZBA
    && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
@@ -110,7 +110,7 @@  (define_insn "*slliuw"
   [(set (match_operand:DI 0 "register_operand" "=r")
 	(and:DI (ashift:DI (match_operand:DI 1 "register_operand" "r")
 			   (match_operand:QI 2 "immediate_operand" "I"))
-		(match_operand 3 "immediate_operand" "")))]
+		(match_operand 3 "immediate_operand" "n")))]
   "TARGET_64BIT && TARGET_ZBA
    && (INTVAL (operands[3]) >> INTVAL (operands[2])) == 0xffffffff"
   "slli.uw\t%0,%1,%2"
@@ -354,6 +354,7 @@  (define_insn "*bexti"
 	(zero_extract:X (match_operand:X 1 "register_operand" "r")
 			(const_int 1)
 			(match_operand 2 "immediate_operand" "i")))]
-  "TARGET_ZBS"
+			(match_operand 2 "immediate_operand" "n")))]
+  "TARGET_ZBS && UINTVAL (operands[2]) < GET_MODE_BITSIZE (<MODE>mode)"
   "bexti\t%0,%1,%2"
   [(set_attr "type" "bitmanip")])