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
