Re: [PATCH] RISC-V: Fix Zicond ICE on large constants

[Jeff Law <jeffreyalaw@gmail.com>]

On 9/4/23 00:45, Kito Cheng wrote:
> 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)
But for the imm, imm case if we force things into regs too early, then 
we'll lose if alt - cons and cons fit in a 12 bit immediate but alt does 
not.

I think Tsukasa is on the right path here.  I should have checked 
riscv_emit_binary -- I though it handled the out-of-range constant case, 
but looking at it now, it clearly does not.

I think this implies we need a similar blob of code for the imm, imm 
case for cons.

Jeff