diff mbox series

RISC-V: Fix build errors with shNadd/shNadd.uw patterns in zba cost model

Message ID alpine.DEB.2.20.2111011214570.3741@tpp.orcam.me.uk
State New
Headers show
Series RISC-V: Fix build errors with shNadd/shNadd.uw patterns in zba cost model | expand

Commit Message

Maciej W. Rozycki Nov. 1, 2021, 12:40 p.m. UTC 
Fix a build regression from commit 04a9b554ba1a ("RISC-V: Cost model 
for zba extension."):

.../gcc/config/riscv/riscv.c: In function 'bool riscv_rtx_costs(rtx, machine_mode, int, int, int*, bool)':
.../gcc/config/riscv/riscv.c:2018:11: error: 'and' of mutually exclusive equal-tests is always 0 [-Werror]
 2018 |           && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 0)), 1, 3))
      |           ^~
.../gcc/config/riscv/riscv.c:2047:17: error: unused variable 'ashift_lhs' [-Werror=unused-variable]
 2047 |             rtx ashift_lhs = XEXP (and_lhs, 0);
      |                 ^~~~~~~~~~


by removing an incorrect REG_P check applied to a constant expression 
and getting rid of the unused variable.

	gcc/
	* config/riscv/riscv.c (riscv_rtx_costs): Remove a REG_P check 
	and an unused local variable with shNadd/shNadd.uw pattern 
	handling.
---
Hi,

 As described above and I guess almost obvious -- I gather the code was 
only verified with a `-Wno-error' build and the handling of the shNadd 
pattern has not been actually covered owing to this bug making the 
condition impossible to match.

 OK to apply then?

  Maciej
---
 gcc/config/riscv/riscv.c |    2 --
 1 file changed, 2 deletions(-)

gcc-riscv-rtx-costs-zba-shnadd.diff

Comments

Kito Cheng Nov. 2, 2021, 3:03 p.m. UTC | #1 
Hi Maciej:

On Mon, Nov 1, 2021 at 8:41 PM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> Fix a build regression from commit 04a9b554ba1a ("RISC-V: Cost model
> for zba extension."):
>
> .../gcc/config/riscv/riscv.c: In function 'bool riscv_rtx_costs(rtx, machine_mode, int, int, int*, bool)':
> .../gcc/config/riscv/riscv.c:2018:11: error: 'and' of mutually exclusive equal-tests is always 0 [-Werror]
>  2018 |           && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 0)), 1, 3))
>       |           ^~
> .../gcc/config/riscv/riscv.c:2047:17: error: unused variable 'ashift_lhs' [-Werror=unused-variable]
>  2047 |             rtx ashift_lhs = XEXP (and_lhs, 0);
>       |                 ^~~~~~~~~~
>
>
> by removing an incorrect REG_P check applied to a constant expression
> and getting rid of the unused variable.
>
>         gcc/
>         * config/riscv/riscv.c (riscv_rtx_costs): Remove a REG_P check
>         and an unused local variable with shNadd/shNadd.uw pattern
>         handling.
> ---
> Hi,
>
>  As described above and I guess almost obvious -- I gather the code was
> only verified with a `-Wno-error' build and the handling of the shNadd
> pattern has not been actually covered owing to this bug making the
> condition impossible to match.
>
>  OK to apply then?
>
>   Maciej
> ---
>  gcc/config/riscv/riscv.c |    2 --
>  1 file changed, 2 deletions(-)
>
> gcc-riscv-rtx-costs-zba-shnadd.diff
> Index: gcc/gcc/config/riscv/riscv.c
> ===================================================================
> --- gcc.orig/gcc/config/riscv/riscv.c
> +++ gcc/gcc/config/riscv/riscv.c
> @@ -2013,7 +2013,6 @@ riscv_rtx_costs (rtx x, machine_mode mod
>           && ((!TARGET_64BIT && (mode == SImode)) ||
>               (TARGET_64BIT && (mode == DImode)))
>           && (GET_CODE (XEXP (x, 0)) == ASHIFT)
> -         && REG_P (XEXP (XEXP (x, 0), 0))
>           && CONST_INT_P (XEXP (XEXP (x, 0), 0))
>           && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 0)), 1, 3))

I think that's my mistake...it should fix following check rather than
remove the REG_P like that:

@@ -2014,8 +2014,8 @@ riscv_rtx_costs (rtx x, machine_mode mode, int
outer_code, int opno ATTRIBUTE_UN
             (TARGET_64BIT && (mode == DImode)))
         && (GET_CODE (XEXP (x, 0)) == ASHIFT)
         && REG_P (XEXP (XEXP (x, 0), 0))
-         && CONST_INT_P (XEXP (XEXP (x, 0), 0))
-         && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 0)), 1, 3))
+         && CONST_INT_P (XEXP (XEXP (x, 0), 1))
+         && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 1)), 1, 3))
       {
         *total = COSTS_N_INSNS (1);
         return true;


shNadd pattern:

(define_insn "*shNadd"
 [(set (match_operand:X 0 "register_operand" "=r")
       (plus:X (ashift:X (match_operand:X 1 "register_operand" "r")
                         # What I want to check is here, it should be
XEXP (XEXP (x, 0), 1)
                         (match_operand:QI 2 "immediate_operand" "I"))
               (match_operand:X 3 "register_operand" "r")))]

Otherwise LGTM, feel free to commit once you address this issue.

Thanks!
diff mbox series

Patch

Index: gcc/gcc/config/riscv/riscv.c
===================================================================
--- gcc.orig/gcc/config/riscv/riscv.c
+++ gcc/gcc/config/riscv/riscv.c
@@ -2013,7 +2013,6 @@  riscv_rtx_costs (rtx x, machine_mode mod
 	  && ((!TARGET_64BIT && (mode == SImode)) ||
 	      (TARGET_64BIT && (mode == DImode)))
 	  && (GET_CODE (XEXP (x, 0)) == ASHIFT)
-	  && REG_P (XEXP (XEXP (x, 0), 0))
 	  && CONST_INT_P (XEXP (XEXP (x, 0), 0))
 	  && IN_RANGE (INTVAL (XEXP (XEXP (x, 0), 0)), 1, 3))
 	{
@@ -2044,7 +2043,6 @@  riscv_rtx_costs (rtx x, machine_mode mod
 	    if (!CONST_INT_P (and_rhs))
 	      break;
 
-	    rtx ashift_lhs = XEXP (and_lhs, 0);
 	    rtx ashift_rhs = XEXP (and_lhs, 1);
 
 	    if (!CONST_INT_P (ashift_rhs)