| Message ID | 20230426173232.2227787-1-tsimpson@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] Hexagon (target/hexagon) Additional instructions handled by idef-parser | expand |
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. diff: Intrinsic: ----------- Test Result Summary ---------- Word32 Q6_R_mpy_RR_rnd(Word32 Rs, Word32 Rt) -40000000 : Q6_R_mpy_RR_rnd(INT32_MIN,INT32_MIN) -1 : Q6_R_mpy_RR_rnd(-1,INT32_MIN) -0 : Q6_R_mpy_RR_rnd(0,INT32_MIN) -0 : Q6_R_mpy_RR_rnd(1,INT32_MIN) -c0000001 : Q6_R_mpy_RR_rnd(INT32_MAX,INT32_MIN) -1 : Q6_R_mpy_RR_rnd(INT32_MIN,-1) -0 : Q6_R_mpy_RR_rnd(-1,-1) -0 : Q6_R_mpy_RR_rnd(0,-1) -0 : Q6_R_mpy_RR_rnd(1,-1) -0 : Q6_R_mpy_RR_rnd(INT32_MAX,-1) -0 : Q6_R_mpy_RR_rnd(INT32_MIN,0) -0 : Q6_R_mpy_RR_rnd(-1,0) -0 : Q6_R_mpy_RR_rnd(0,0) -0 : Q6_R_mpy_RR_rnd(1,0) -0 : Q6_R_mpy_RR_rnd(INT32_MAX,0) -0 : Q6_R_mpy_RR_rnd(INT32_MIN,1) -0 : Q6_R_mpy_RR_rnd(-1,1) -0 : Q6_R_mpy_RR_rnd(0,1) -0 : Q6_R_mpy_RR_rnd(1,1) -0 : Q6_R_mpy_RR_rnd(INT32_MAX,1) -c0000001 : Q6_R_mpy_RR_rnd(INT32_MIN,INT32_MAX) -0 : Q6_R_mpy_RR_rnd(-1,INT32_MAX) -0 : Q6_R_mpy_RR_rnd(0,INT32_MAX) -0 : Q6_R_mpy_RR_rnd(1,INT32_MAX) -3fffffff : Q6_R_mpy_RR_rnd(INT32_MAX,INT32_MAX) +3fffffff : Q6_R_mpy_RR_rnd(INT32_MIN,INT32_MIN) +0 : Q6_R_mpy_RR_rnd(-1,INT32_MIN) +ffffffff : Q6_R_mpy_RR_rnd(0,INT32_MIN) +ffffffff : Q6_R_mpy_RR_rnd(1,INT32_MIN) +c0000000 : Q6_R_mpy_RR_rnd(INT32_MAX,INT32_MIN) +0 : Q6_R_mpy_RR_rnd(INT32_MIN,-1) +ffffffff : Q6_R_mpy_RR_rnd(-1,-1) +ffffffff : Q6_R_mpy_RR_rnd(0,-1) +ffffffff : Q6_R_mpy_RR_rnd(1,-1) +ffffffff : Q6_R_mpy_RR_rnd(INT32_MAX,-1) +ffffffff : Q6_R_mpy_RR_rnd(INT32_MIN,0) +ffffffff : Q6_R_mpy_RR_rnd(-1,0) +ffffffff : Q6_R_mpy_RR_rnd(0,0) +ffffffff : Q6_R_mpy_RR_rnd(1,0) +ffffffff : Q6_R_mpy_RR_rnd(INT32_MAX,0) +ffffffff : Q6_R_mpy_RR_rnd(INT32_MIN,1) +ffffffff : Q6_R_mpy_RR_rnd(-1,1) +ffffffff : Q6_R_mpy_RR_rnd(0,1) +ffffffff : Q6_R_mpy_RR_rnd(1,1) +ffffffff : Q6_R_mpy_RR_rnd(INT32_MAX,1) +c0000000 : Q6_R_mpy_RR_rnd(INT32_MIN,INT32_MAX) +ffffffff : Q6_R_mpy_RR_rnd(-1,INT32_MAX) +ffffffff : Q6_R_mpy_RR_rnd(0,INT32_MAX) +ffffffff : Q6_R_mpy_RR_rnd(1,INT32_MAX) +3ffffffe : Q6_R_mpy_RR_rnd(INT32_MAX,INT32_MAX)
> -----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. Taylor
> -----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
diff --git a/target/hexagon/idef-parser/parser-helpers.h b/target/hexagon/idef-parser/parser-helpers.h index 1239d23a6a..7c58087169 100644 --- a/target/hexagon/idef-parser/parser-helpers.h +++ b/target/hexagon/idef-parser/parser-helpers.h @@ -80,7 +80,7 @@ void reg_compose(Context *c, YYLTYPE *locp, HexReg *reg, char reg_id[5]); void reg_print(Context *c, YYLTYPE *locp, HexReg *reg); -void imm_print(Context *c, YYLTYPE *locp, HexImm *imm); +void imm_print(Context *c, YYLTYPE *locp, HexValue *rvalue); void var_print(Context *c, YYLTYPE *locp, HexVar *var); diff --git a/target/hexagon/idef-parser/parser-helpers.c b/target/hexagon/idef-parser/parser-helpers.c index 86511efb62..deaedbb293 100644 --- a/target/hexagon/idef-parser/parser-helpers.c +++ b/target/hexagon/idef-parser/parser-helpers.c @@ -167,8 +167,9 @@ void reg_print(Context *c, YYLTYPE *locp, HexReg *reg) EMIT(c, "hex_gpr[%u]", reg->id); } -void imm_print(Context *c, YYLTYPE *locp, HexImm *imm) +void imm_print(Context *c, YYLTYPE *locp, HexValue *rvalue) { + HexImm *imm = &rvalue->imm; switch (imm->type) { case I: EMIT(c, "i"); @@ -177,7 +178,21 @@ void imm_print(Context *c, YYLTYPE *locp, HexImm *imm) EMIT(c, "%ciV", imm->id); break; case VALUE: - EMIT(c, "((int64_t) %" PRIu64 "ULL)", (int64_t) imm->value); + if (rvalue->bit_width == 32) { + if (rvalue->signedness == UNSIGNED) { + EMIT(c, "((uint32_t) %" PRIu32 ")", (uint32_t) imm->value); + } else { + EMIT(c, "((int32_t) %" PRId32 ")", (int32_t) imm->value); + } + } else if (rvalue->bit_width == 64) { + if (rvalue->signedness == UNSIGNED) { + EMIT(c, "((uint64_t) %" PRIu64 "ULL)", (uint64_t) imm->value); + } else { + EMIT(c, "((int64_t) %" PRId64 "LL)", (int64_t) imm->value); + } + } else { + g_assert_not_reached(); + } break; case QEMU_TMP: EMIT(c, "qemu_tmp_%" PRIu64, imm->index); @@ -213,7 +228,7 @@ void rvalue_print(Context *c, YYLTYPE *locp, void *pointer) tmp_print(c, locp, &rvalue->tmp); break; case IMMEDIATE: - imm_print(c, locp, &rvalue->imm); + imm_print(c, locp, rvalue); break; case VARID: var_print(c, locp, &rvalue->var); @@ -386,13 +401,10 @@ HexValue gen_rvalue_extend(Context *c, YYLTYPE *locp, HexValue *rvalue) if (rvalue->type == IMMEDIATE) { HexValue res = gen_imm_qemu_tmp(c, locp, 64, rvalue->signedness); - bool is_unsigned = (rvalue->signedness == UNSIGNED); - const char *sign_suffix = is_unsigned ? "u" : ""; gen_c_int_type(c, locp, 64, rvalue->signedness); - OUT(c, locp, " ", &res, " = "); - OUT(c, locp, "(", sign_suffix, "int64_t) "); - OUT(c, locp, "(", sign_suffix, "int32_t) "); - OUT(c, locp, rvalue, ";\n"); + OUT(c, locp, " ", &res, " = ("); + gen_c_int_type(c, locp, 64, rvalue->signedness); + OUT(c, locp, ")", rvalue, ";\n"); return res; } else { HexValue res = gen_tmp(c, locp, 64, rvalue->signedness); @@ -963,7 +975,12 @@ HexValue gen_cast_op(Context *c, if (src->bit_width == target_width) { return *src; } else if (src->type == IMMEDIATE) { - HexValue res = *src; + HexValue res; + if (src->bit_width < target_width) { + res = gen_rvalue_extend(c, locp, src); + } else { + res = *src; + } res.bit_width = target_width; res.signedness = signedness; return res; diff --git a/tests/tcg/hexagon/fpstuff.c b/tests/tcg/hexagon/fpstuff.c index 90ce9a6ef3..28f9397155 100644 --- a/tests/tcg/hexagon/fpstuff.c +++ b/tests/tcg/hexagon/fpstuff.c @@ -20,6 +20,7 @@ */ #include <stdio.h> +#include <float.h> const int FPINVF_BIT = 1; /* Invalid */ const int FPINVF = 1 << FPINVF_BIT; @@ -706,6 +707,57 @@ static void check_float2int_convs() check_fpstatus(usr, FPINVF); } +static void check_float_consts(void) +{ + int res32; + unsigned long long res64; + + asm("%0 = sfmake(#%1):neg\n\t" : "=r"(res32) : "i"(0xf)); + check32(res32, 0xbc9e0000); + + asm("%0 = sfmake(#%1):pos\n\t" : "=r"(res32) : "i"(0xf)); + check32(res32, 0x3c9e0000); + + asm("%0 = dfmake(#%1):neg\n\t" : "=r"(res64) : "i"(0xf)); + check64(res64, 0xbf93c00000000000ULL); + + asm("%0 = dfmake(#%1):pos\n\t" : "=r"(res64) : "i"(0xf)); + check64(res64, 0x3f93c00000000000ULL); +} + +static inline unsigned long long dfmpyll(double x, double y) +{ + unsigned long long res64; + asm("%0 = dfmpyll(%1, %2)" : "=r"(res64) : "r"(x), "r"(y)); + return res64; +} + +static inline unsigned long long dfmpylh(double acc, double x, double y) +{ + unsigned long long res64 = *(unsigned long long *)&acc; + asm("%0 += dfmpylh(%1, %2)" : "+r"(res64) : "r"(x), "r"(y)); + return res64; +} + +static void check_dfmpyxx(void) +{ + unsigned long long res64; + + res64 = dfmpyll(DBL_MIN, DBL_MIN); + check64(res64, 0ULL); + res64 = dfmpyll(-1.0, DBL_MIN); + check64(res64, 0ULL); + res64 = dfmpyll(DBL_MAX, DBL_MAX); + check64(res64, 0x1fffffffdULL); + + res64 = dfmpylh(DBL_MIN, DBL_MIN, DBL_MIN); + check64(res64, 0x10000000000000ULL); + res64 = dfmpylh(-1.0, DBL_MAX, DBL_MIN); + check64(res64, 0xc00fffffffe00000ULL); + res64 = dfmpylh(DBL_MAX, 0.0, -1.0); + check64(res64, 0x7fefffffffffffffULL); +} + int main() { check_compare_exception(); @@ -718,6 +770,8 @@ int main() check_sffixupd(); check_sffms(); check_float2int_convs(); + check_float_consts(); + check_dfmpyxx(); puts(err ? "FAIL" : "PASS"); return err ? 1 : 0; diff --git a/target/hexagon/gen_idef_parser_funcs.py b/target/hexagon/gen_idef_parser_funcs.py index afe68bdb6f..b766798ad5 100644 --- a/target/hexagon/gen_idef_parser_funcs.py +++ b/target/hexagon/gen_idef_parser_funcs.py @@ -103,7 +103,15 @@ def main(): continue if tag.startswith("V6_"): continue - if tag.startswith("F"): + if ( tag.startswith("F") and + tag not in { + "F2_sfimm_p", + "F2_sfimm_n", + "F2_dfimm_p", + "F2_dfimm_n", + "F2_dfmpyll", + "F2_dfmpylh" + }): continue if tag.endswith("_locked"): continue
**** 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(-)