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

[Tsukasa OI <research_trasio@irq.a4lg.com>]

On 2023/09/05 14:27, Jeff Law wrote:
> 
> 
> 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
> 

Okay, adding a check to "imm, imm" case (although I haven't figured out
how to reproduce this case) and will submit the v2.

Tsukasa