RE: [PATCH v2] Hexagon (target/hexagon) Additional instructions handled by idef-parser

[Taylor Simpson]

> -----Original Message-----
> From: Taylor Simpson
> Sent: Friday, April 28, 2023 3:53 PM
> To: anjo@rev.ng; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: RE: [PATCH v2] Hexagon (target/hexagon) Additional instructions
> handled by idef-parser
> 
> 
> 
> > -----Original Message-----
> > From: Anton Johansson <anjo@rev.ng>
> > Sent: Friday, April 28, 2023 11:25 AM
> > To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> > Cc: richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng; Brian
> Cain
> > <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> > <quic_mathbern@quicinc.com>
> > Subject: Re: [PATCH v2] Hexagon (target/hexagon) Additional instructions
> > handled by idef-parser
> >
> > On 4/26/23 19:32, Taylor Simpson wrote:
> > > **** Changes in v2 ****
> > > Fix bug in imm_print identified in clang build
> > >
> > > Currently, idef-parser skips all floating point instructions.
> > > However, there are some floating point instructions that can be handled.
> > >
> > > The following instructions are now parsed
> > >      F2_sfimm_p
> > >      F2_sfimm_n
> > >      F2_dfimm_p
> > >      F2_dfimm_n
> > >      F2_dfmpyll
> > >      F2_dfmpylh
> > >
> > > To make these instructions work, we fix some bugs in parser-helpers.c
> > >      gen_rvalue_extend
> > >      gen_cast_op
> > >      imm_print
> > >
> > > Test cases added to tests/tcg/hexagon/fpstuff.c
> > >
> > > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> > > ---
> > >   target/hexagon/idef-parser/parser-helpers.h |  2 +-
> > >   target/hexagon/idef-parser/parser-helpers.c | 37 ++++++++++----
> > >   tests/tcg/hexagon/fpstuff.c                 | 54 +++++++++++++++++++++
> > >   target/hexagon/gen_idef_parser_funcs.py     | 10 +++-
> > >   4 files changed, 91 insertions(+), 12 deletions(-)
> >
> > I'm getting a harness failure on
> >
> >      v65_Q6_R_mpy_RR_rnd.c
> >
> > I'll take a deeper look at this next week.
> 
> I'm seeing that failure too.  Thanks for looking into it.

It's this instruction
void emit_M2_dpmpyss_rnd_s0(DisasContext * ctx, Insn * insn, Packet * pkt,
                            TCGv_i32 RdV, TCGv_i32 RsV, TCGv_i32 RtV)
/* {RdV=(fMPY32SS(RsV,RtV)+fCONSTLL(0x80000000))>>32;} */ {
        TCGv_i64 tmp_0 = tcg_temp_new_i64();
        tcg_gen_ext_i32_i64(tmp_0, RsV);
        TCGv_i64 tmp_1 = tcg_temp_new_i64();
        tcg_gen_ext_i32_i64(tmp_1, RtV);
        TCGv_i64 tmp_2 = tcg_temp_new_i64();
        tcg_gen_mul_i64(tmp_2, tmp_0, tmp_1);
        int64_t qemu_tmp_0 = (int64_t) ((int32_t) - 2147483648);
        TCGv_i64 tmp_3 = tcg_temp_new_i64();
        tcg_gen_addi_i64(tmp_3, tmp_2, qemu_tmp_0);
        int64_t qemu_tmp_1 = (int64_t) ((int32_t) 32);
        TCGv_i64 tmp_4 = tcg_temp_new_i64();
        {
                int64_t shift = qemu_tmp_1;
                if (qemu_tmp_1 >= 64) {
                        shift = 64 - 1;
                }
                tcg_gen_sari_i64(tmp_4, tmp_3, shift);
        }
        TCGv_i32 tmp_5 = tcg_temp_new_i32();
        tcg_gen_trunc_i64_tl(tmp_5, tmp_4);
        tcg_gen_mov_i32(RdV, tmp_5);
}

The problem is how we handle fCONSTLL(0x80000000).  In macros.h, it's
    #define fCONSTLL(A) A##LL

The parser is treating it as a cast to int64_t.  However,
     0x80000000LL != (int64_t) 0x80000000

I'll change fCONSTLL from a cast to simply changing the bit_width to 64 and 
signedness to SIGNED.

Stay tuned for v3 of the patch.

Thanks,
Taylor