Re: [RFC PATCH 2/3] tcg/riscv: Fix tcg_out_opc_imm when imm exceeds

[Richard Henderson <richard.henderson@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]

On Fri, 21 Oct 2022, 12:57 LIU Zhiwei, <zhiwei_liu@linux.alibaba.com> wrote:

>
> On 2022/10/20 19:22, Richard Henderson wrote:
> > On 10/20/22 20:41, LIU Zhiwei wrote:
> >> TYPE-I immediate can only represent a signed 12-bit value. If immediate
> >> exceed, mov it to an register.
> >>
> >> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> >> ---
> >>   tcg/riscv/tcg-target.c.inc | 28 +++++++++++++++++++++++-----
> >>   1 file changed, 23 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
> >> index 32f4bc7bfc..bfdf2bea69 100644
> >> --- a/tcg/riscv/tcg-target.c.inc
> >> +++ b/tcg/riscv/tcg-target.c.inc
> >> @@ -668,7 +668,12 @@ static void tcg_out_addsub2(TCGContext *s,
> >>       if (!cbh) {
> >>           tcg_out_opc_reg(s, (is_sub ? opc_sub : opc_add), th, ah, bh);
> >>       } else if (bh != 0 || ah == rl) {
> >> -        tcg_out_opc_imm(s, opc_addi, th, ah, (is_sub ? -bh : bh));
> >> +        if (bh == sextract(bh, 0, 12)) {
> >> +            tcg_out_opc_imm(s, opc_addi, th, ah, (is_sub ? -bh : bh));
> >> +        } else {
> >> +            tcg_out_movi(s, TCG_TYPE_TL, th, (is_sub ? -bh : bh));
> >> +            tcg_out_opc_reg(s, opc_add, th, ah, th);
> >> +        }
> >
> > This value is currently constrained by 'M': +/- 0xfff.
>
> I don't know why we need 'M'. Can I just use the constraint
>
> C_O2_I4(r, r, rZ, rZ, rS, rS);
>

I see the problem now. Look at the top of tcg_out_addsub2, where we
(conditionally) negate the constant.

We want to constrain the constant to be representable either positive or
negative, i.e not -4096..4095 but -4095..4095.  But we got the endpoints
wrong in tcg_target_const_match: 0xfff instead of 0x7ff.


r~

[-- Attachment #2: Type: text/html, Size: 2615 bytes --]