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

Taylor Simpson posted 1 patch 12 months ago
Failed in applying to current master (apply log)
Maintainers: Alessandro Di Federico <ale@rev.ng>, Anton Johansson <anjo@rev.ng>, Taylor Simpson <tsimpson@quicinc.com>
target/hexagon/idef-parser/parser-helpers.h |  2 +-
target/hexagon/idef-parser/parser-helpers.c | 41 +++++++++++-----
tests/tcg/hexagon/fpstuff.c                 | 54 +++++++++++++++++++++
tests/tcg/hexagon/misc.c                    | 35 +++++++++++++
target/hexagon/gen_idef_parser_funcs.py     | 10 +++-
target/hexagon/idef-parser/idef-parser.lex  | 38 +++++++++++++--
target/hexagon/idef-parser/idef-parser.y    |  2 -
7 files changed, 162 insertions(+), 20 deletions(-)
[PATCH v3] Hexagon (target/hexagon) Additional instructions handled by idef-parser
Posted by Taylor Simpson 12 months ago
**** Changes in v3 ****
Fix bugs exposed by dpmpyss_rnd_s0 instruction
    Set correct size/signedness for constants
    Test cases added to tests/tcg/hexagon/misc.c

**** 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
    lexer properly sets size/signedness of constants

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 | 41 +++++++++++-----
 tests/tcg/hexagon/fpstuff.c                 | 54 +++++++++++++++++++++
 tests/tcg/hexagon/misc.c                    | 35 +++++++++++++
 target/hexagon/gen_idef_parser_funcs.py     | 10 +++-
 target/hexagon/idef-parser/idef-parser.lex  | 38 +++++++++++++--
 target/hexagon/idef-parser/idef-parser.y    |  2 -
 7 files changed, 162 insertions(+), 20 deletions(-)

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..0ad917f591 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) 0x%" PRIx32 ")", (uint32_t) imm->value);
+            }  else {
+                EMIT(c, "((int32_t) 0x%" PRIx32 ")", (int32_t) imm->value);
+            }
+        } else if (rvalue->bit_width == 64) {
+            if (rvalue->signedness == UNSIGNED) {
+                EMIT(c, "((uint64_t) 0x%" PRIx64 "ULL)", (uint64_t) imm->value);
+            } else {
+                EMIT(c, "((int64_t) 0x%" PRIx64 "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);
@@ -961,9 +973,16 @@ HexValue gen_cast_op(Context *c,
 {
     assert_signedness(c, locp, src->signedness);
     if (src->bit_width == target_width) {
-        return *src;
-    } else if (src->type == IMMEDIATE) {
         HexValue res = *src;
+        res.signedness = signedness;
+        return res;
+    } else if (src->type == IMMEDIATE) {
+        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/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index e126751e3a..4c994c55bd 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -385,6 +385,39 @@ void test_count_trailing_zeros_ones(void)
     check(ct1p(0xffffff0fffffffffULL), 36);
 }
 
+static inline int dpmpyss_rnd_s0(int x, int y)
+{
+    int res;
+    asm("%0 = mpy(%1, %2):rnd\n\t" : "=r"(res) : "r"(x), "r"(y));
+    return res;
+}
+
+void test_dpmpyss_rnd_s0(void)
+{
+    check(dpmpyss_rnd_s0(-1, 0x80000000), 1);
+    check(dpmpyss_rnd_s0(0, 0x80000000), 0);
+    check(dpmpyss_rnd_s0(1, 0x80000000), 0);
+    check(dpmpyss_rnd_s0(0x7fffffff, 0x80000000), 0xc0000001);
+    check(dpmpyss_rnd_s0(0x80000000, -1), 1);
+    check(dpmpyss_rnd_s0(-1, -1), 0);
+    check(dpmpyss_rnd_s0(0, -1), 0);
+    check(dpmpyss_rnd_s0(1, -1), 0);
+    check(dpmpyss_rnd_s0(0x7fffffff, -1), 0);
+    check(dpmpyss_rnd_s0(0x80000000, 0), 0);
+    check(dpmpyss_rnd_s0(-1, 0), 0);
+    check(dpmpyss_rnd_s0(0, 0), 0);
+    check(dpmpyss_rnd_s0(1, 0), 0);
+    check(dpmpyss_rnd_s0(-1, -1), 0);
+    check(dpmpyss_rnd_s0(0, -1), 0);
+    check(dpmpyss_rnd_s0(1, -1), 0);
+    check(dpmpyss_rnd_s0(0x7fffffff, 1), 0);
+    check(dpmpyss_rnd_s0(0x80000000, 0x7fffffff), 0xc0000001);
+    check(dpmpyss_rnd_s0(-1, 0x7fffffff), 0);
+    check(dpmpyss_rnd_s0(0, 0x7fffffff),  0);
+    check(dpmpyss_rnd_s0(1, 0x7fffffff),  0);
+    check(dpmpyss_rnd_s0(0x7fffffff, 0x7fffffff), 0x3fffffff);
+}
+
 int main()
 {
     int res;
@@ -522,6 +555,8 @@ int main()
 
     test_count_trailing_zeros_ones();
 
+    test_dpmpyss_rnd_s0();
+
     puts(err ? "FAIL" : "PASS");
     return err;
 }
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
diff --git a/target/hexagon/idef-parser/idef-parser.lex b/target/hexagon/idef-parser/idef-parser.lex
index 5eb8ac5a80..4b81e63a02 100644
--- a/target/hexagon/idef-parser/idef-parser.lex
+++ b/target/hexagon/idef-parser/idef-parser.lex
@@ -401,12 +401,40 @@ STRING_LIT               \"(\\.|[^"\\])*\"
                            }
                            return SIGN;
                          }
-"0x"{HEX_DIGIT}+         |
-{DIGIT}+                 { yylval->rvalue.type = IMMEDIATE;
-                           yylval->rvalue.bit_width = 32;
-                           yylval->rvalue.signedness = SIGNED;
+"0x"{HEX_DIGIT}+         { uint64_t value = strtoull(yytext, NULL, 0);
+                           yylval->rvalue.type = IMMEDIATE;
                            yylval->rvalue.imm.type = VALUE;
-                           yylval->rvalue.imm.value = strtoull(yytext, NULL, 0);
+                           yylval->rvalue.imm.value = value;
+                           if (value <= INT_MAX) {
+                               yylval->rvalue.bit_width = sizeof(int) * 8;
+                               yylval->rvalue.signedness = SIGNED;
+                           } else if (value <= UINT_MAX) {
+                               yylval->rvalue.bit_width = sizeof(unsigned int) * 8;
+                               yylval->rvalue.signedness = UNSIGNED;
+                           } else if (value <= LONG_MAX) {
+                               yylval->rvalue.bit_width = sizeof(long) * 8;
+                               yylval->rvalue.signedness = SIGNED;
+                           } else if (value <= ULONG_MAX) {
+                               yylval->rvalue.bit_width = sizeof(unsigned long) * 8;
+                               yylval->rvalue.signedness = UNSIGNED;
+                           } else {
+                               g_assert_not_reached();
+                           }
+                           return IMM; }
+{DIGIT}+                 { int64_t value = strtoll(yytext, NULL, 0);
+                           yylval->rvalue.type = IMMEDIATE;
+                           yylval->rvalue.imm.type = VALUE;
+                           yylval->rvalue.imm.value = value;
+                           if (value >= INT_MIN && value <= INT_MAX) {
+                               yylval->rvalue.bit_width = sizeof(int) * 8;
+                               yylval->rvalue.signedness = SIGNED;
+                           } else if (value >= LONG_MIN && value <= LONG_MAX) {
+                               yylval->rvalue.bit_width = sizeof(long) * 8;
+                               yylval->rvalue.signedness = SIGNED;
+                           } else {
+                              printf("value = 0x%lx\n", value);
+                              g_assert_not_reached();
+                           }
                            return IMM; }
 "0x"{HEX_DIGIT}+"ULL"    |
 {DIGIT}+"ULL"            { yylval->rvalue.type = IMMEDIATE;
diff --git a/target/hexagon/idef-parser/idef-parser.y b/target/hexagon/idef-parser/idef-parser.y
index 5444fd4749..5f3907eb28 100644
--- a/target/hexagon/idef-parser/idef-parser.y
+++ b/target/hexagon/idef-parser/idef-parser.y
@@ -594,8 +594,6 @@ rvalue : FAIL
        | CAST rvalue
          {
              @1.last_column = @2.last_column;
-             /* Assign target signedness */
-             $2.signedness = $1.signedness;
              $$ = gen_cast_op(c, &@1, &$2, $1.bit_width, $1.signedness);
          }
        | rvalue EQ rvalue
-- 
2.25.1

Re: [PATCH v3] Hexagon (target/hexagon) Additional instructions handled by idef-parser
Posted by Anton Johansson via 12 months ago
On 5/1/23 22:31, Taylor Simpson wrote:
> **** Changes in v3 ****
> Fix bugs exposed by dpmpyss_rnd_s0 instruction
>      Set correct size/signedness for constants
>      Test cases added to tests/tcg/hexagon/misc.c
>
> **** 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
>      lexer properly sets size/signedness of constants
>
> 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 | 41 +++++++++++-----
>   tests/tcg/hexagon/fpstuff.c                 | 54 +++++++++++++++++++++
>   tests/tcg/hexagon/misc.c                    | 35 +++++++++++++
>   target/hexagon/gen_idef_parser_funcs.py     | 10 +++-
>   target/hexagon/idef-parser/idef-parser.lex  | 38 +++++++++++++--
>   target/hexagon/idef-parser/idef-parser.y    |  2 -
>   7 files changed, 162 insertions(+), 20 deletions(-)
>
> 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..0ad917f591 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) 0x%" PRIx32 ")", (uint32_t) imm->value);
> +            }  else {
> +                EMIT(c, "((int32_t) 0x%" PRIx32 ")", (int32_t) imm->value);
> +            }
> +        } else if (rvalue->bit_width == 64) {
> +            if (rvalue->signedness == UNSIGNED) {
> +                EMIT(c, "((uint64_t) 0x%" PRIx64 "ULL)", (uint64_t) imm->value);
> +            } else {
> +                EMIT(c, "((int64_t) 0x%" PRIx64 "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);
> @@ -961,9 +973,16 @@ HexValue gen_cast_op(Context *c,
>   {
>       assert_signedness(c, locp, src->signedness);
>       if (src->bit_width == target_width) {
> -        return *src;
> -    } else if (src->type == IMMEDIATE) {
>           HexValue res = *src;
> +        res.signedness = signedness;
> +        return res;
> +    } else if (src->type == IMMEDIATE) {
> +        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;

Ah, gen_cast_op() can be simplified a great deal here to

     HexValue gen_cast_op(Context *c,
                          YYLTYPE *locp,
                          HexValue *src,
                          unsigned target_width,
                          HexSignedness signedness)
     {
         HexValue res;
         assert_signedness(c, locp, src->signedness);
         if (src->bit_width == target_width) {
             res = *src;
         } else if (src->bit_width < target_width) {
             res = gen_rvalue_extend(c, locp, src);
         } else {
             res = gen_rvalue_truncate(c, locp, src);
         }
         res.signedness = signedness;
         return res;
     }

Other than that this patch looks good:

Tested-by: Anton Johansson <anjo@rev.ng>
Reviewed-by: Anton Johansson <anjo@rev.ng>



RE: [PATCH v3] Hexagon (target/hexagon) Additional instructions handled by idef-parser
Posted by Taylor Simpson 12 months ago

> -----Original Message-----
> From: Anton Johansson <anjo@rev.ng>
> Sent: Tuesday, May 2, 2023 6:12 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 v3] Hexagon (target/hexagon) Additional instructions
> handled by idef-parser
> 
> On 5/1/23 22:31, Taylor Simpson wrote:
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index 86511efb62..0ad917f591 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > -961,9 +973,16 @@ HexValue gen_cast_op(Context *c,
> >   {
> >       assert_signedness(c, locp, src->signedness);
> >       if (src->bit_width == target_width) {
> > -        return *src;
> > -    } else if (src->type == IMMEDIATE) {
> >           HexValue res = *src;
> > +        res.signedness = signedness;
> > +        return res;
> > +    } else if (src->type == IMMEDIATE) {
> > +        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;
> 
> Ah, gen_cast_op() can be simplified a great deal here to
> 
>      HexValue gen_cast_op(Context *c,
>                           YYLTYPE *locp,
>                           HexValue *src,
>                           unsigned target_width,
>                           HexSignedness signedness)
>      {
>          HexValue res;
>          assert_signedness(c, locp, src->signedness);
>          if (src->bit_width == target_width) {
>              res = *src;
>          } else if (src->bit_width < target_width) {
>              res = gen_rvalue_extend(c, locp, src);
>          } else {
>              res = gen_rvalue_truncate(c, locp, src);
>          }
>          res.signedness = signedness;
>          return res;
>      }

Great suggestion, thanks!

> 
> Other than that this patch looks good:
> 
> Tested-by: Anton Johansson <anjo@rev.ng>
> Reviewed-by: Anton Johansson <anjo@rev.ng>
>