Maybe move the check logic a bit forward? My thought is the logic is
already specialized into a few catalogs, (imm, imm), (imm, reg), (reg,
reg)... and the logic you put is already in (imm, reg), but it should
really move into (reg, reg) case IMO? and move that forward we could
prevent add too much logic to redirect the case.

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 2db9c81ac8b..c84509c393b 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -3892,6 +3892,12 @@ riscv_expand_conditional_move (rtx dest, rtx
op, rtx cons, rtx alt)
         op1 = XEXP (op, 1);
       }

+      /* CONS might not fit into a signed 12 bit immediate suitable
+        for an addi instruction.  If that's the case, force it into
+        a register.  */
+      if (CONST_INT_P (cons) && !SMALL_OPERAND (INTVAL (cons)))
+       cons = force_reg (mode, cons);
+
      /* 0, reg or 0, imm */
      if (cons == CONST0_RTX (mode)
         && (REG_P (alt)

On Mon, Sep 4, 2023 at 8:21 AM Tsukasa OI via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Tsukasa OI <research_tra...@irq.a4lg.com>
>
> Large constant cons and/or alt will trigger ICEs building GCC target
> libraries (libgomp and libatomic) when the 'Zicond' extension is enabled.
>
> For instance, zicond-ice-2.c (new test case in this commit) will cause
> an ICE when SOME_NUMBER is 0x1000 or larger.  While opposite numbers
> corresponding cons/alt (two temp2 variables) are checked, cons/alt
> themselves are not checked and causing 2 ICEs building
> GCC target libraries as of this writing:
>
> 1.  gcc/libatomic/config/posix/lock.c
> 2.  gcc/libgomp/fortran.c
>
> Coercing a large value into a register will fix the issue.
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_expand_conditional_move): Force
>         large constant cons/alt into a register.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/zicond-ice-2.c: New test.  This is based on
>         an ICE at libat_lock_n func on gcc/libatomic/config/posix/lock.c
>         but heavily minimized.
> ---
>  gcc/config/riscv/riscv.cc                     | 16 ++++++++++------
>  gcc/testsuite/gcc.target/riscv/zicond-ice-2.c | 11 +++++++++++
>  2 files changed, 21 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/zicond-ice-2.c
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 8d8f7b4f16ed..cfaa4b6a7720 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -3940,11 +3940,13 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx 
> cons, rtx alt)
>           rtx temp1 = gen_reg_rtx (mode);
>           rtx temp2 = gen_int_mode (-1 * INTVAL (cons), mode);
>
> -         /* TEMP2 might not fit into a signed 12 bit immediate suitable
> -            for an addi instruction.  If that's the case, force it into
> -            a register.  */
> +         /* TEMP2 and/or CONS might not fit into a signed 12 bit immediate
> +            suitable for an addi instruction.  If that's the case, force it
> +            into a register.  */
>           if (!SMALL_OPERAND (INTVAL (temp2)))
>             temp2 = force_reg (mode, temp2);
> +         if (!SMALL_OPERAND (INTVAL (cons)))
> +           cons = force_reg (mode, cons);
>
>           riscv_emit_binary (PLUS, temp1, alt, temp2);
>           emit_insn (gen_rtx_SET (dest,
> @@ -3986,11 +3988,13 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx 
> cons, rtx alt)
>           rtx temp1 = gen_reg_rtx (mode);
>           rtx temp2 = gen_int_mode (-1 * INTVAL (alt), mode);
>
> -         /* TEMP2 might not fit into a signed 12 bit immediate suitable
> -            for an addi instruction.  If that's the case, force it into
> -            a register.  */
> +         /* TEMP2 and/or ALT might not fit into a signed 12 bit immediate
> +            suitable for an addi instruction.  If that's the case, force it
> +            into a register.  */
>           if (!SMALL_OPERAND (INTVAL (temp2)))
>             temp2 = force_reg (mode, temp2);
> +         if (!SMALL_OPERAND (INTVAL (alt)))
> +           alt = force_reg (mode, alt);
>
>           riscv_emit_binary (PLUS, temp1, cons, temp2);
>           emit_insn (gen_rtx_SET (dest,
> diff --git a/gcc/testsuite/gcc.target/riscv/zicond-ice-2.c 
> b/gcc/testsuite/gcc.target/riscv/zicond-ice-2.c
> new file mode 100644
> index 000000000000..ffd8dcb5814e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/zicond-ice-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zicond -mabi=lp64d" { target { rv64 } } } */
> +/* { dg-options "-march=rv32gc_zicond -mabi=ilp32d" { target { rv32 } } } */
> +
> +#define SOME_NUMBER 0x1000
> +
> +unsigned long
> +d (unsigned long n)
> +{
> +  return n > SOME_NUMBER ? SOME_NUMBER : n;
> +}
>
> base-commit: 78f636d979530c8a649262dbd44914bdfb6f7290
> --
> 2.42.0
>

Reply via email to