[PATCH v2 2/3] Hexagon (target/hexagon) Use QEMU decodetree (16-bit instructions)

Taylor Simpson posted 3 patches 10 months, 3 weeks ago
Maintainers: Brian Cain <bcain@quicinc.com>
There is a newer version of this series
[PATCH v2 2/3] Hexagon (target/hexagon) Use QEMU decodetree (16-bit instructions)
Posted by Taylor Simpson 10 months, 3 weeks ago
Section 10.3 of the Hexagon V73 Programmer's Reference Manual

A duplex is encoded as a 32-bit instruction with bits [15:14] set to 00.
The sub-instructions that comprise a duplex are encoded as 13-bit fields
in the duplex.

Create a decoder for each subinstruction class (a, l1, l2, s1, s2).

Extend gen_trans_funcs.py to handle all instructions rather than
filter by instruction class.

There is a g_assert_not_reached() in decode_insns() in decode.c to
verify we never try to use the old decoder on 16-bit instructions.

Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
 target/hexagon/decode.c           | 85 +++++++++++++++++++++++++++++
 target/hexagon/README             |  1 +
 target/hexagon/gen_decodetree.py  | 14 ++++-
 target/hexagon/gen_trans_funcs.py | 12 +----
 target/hexagon/meson.build        | 90 +++++++++++++++++++++++++++++++
 5 files changed, 190 insertions(+), 12 deletions(-)

diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index bddad1f75e..160b23a895 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -60,6 +60,7 @@ static int decode_mapped_reg_##NAME(DisasContext *ctx, int x) \
 }
 DECODE_MAPPED(R_16)
 DECODE_MAPPED(R_8)
+DECODE_MAPPED(R__8)
 
 /* Helper function for decodetree_trans_funcs_generated.c.inc */
 static int shift_left(DisasContext *ctx, int x, int n, int immno)
@@ -77,6 +78,13 @@ static int shift_left(DisasContext *ctx, int x, int n, int immno)
 #include "decode_normal_generated.c.inc"
 #include "decode_hvx_generated.c.inc"
 
+/* Include the generated decoder for 16 bit insn */
+#include "decode_subinsn_a_generated.c.inc"
+#include "decode_subinsn_l1_generated.c.inc"
+#include "decode_subinsn_l2_generated.c.inc"
+#include "decode_subinsn_s1_generated.c.inc"
+#include "decode_subinsn_s2_generated.c.inc"
+
 /* Include the generated helpers for the decoder */
 #include "decodetree_trans_funcs_generated.c.inc"
 
@@ -790,6 +798,63 @@ decode_insns_tablewalk(Insn *insn, const DectreeTable *table,
     }
 }
 
+/*
+ * Section 10.3 of the Hexagon V73 Programmer's Reference Manual
+ *
+ * A duplex is encoded as a 32-bit instruction with bits [15:14] set to 00.
+ * The sub-instructions that comprise a duplex are encoded as 13-bit fields
+ * in the duplex.
+ *
+ * Per table 10-4, the 4-bit duplex iclass is encoded in bits 31:29, 13
+ */
+static uint32_t get_duplex_iclass(uint32_t encoding)
+{
+    uint32_t iclass = extract32(encoding, 13, 1);
+    iclass = deposit32(iclass, 1, 3, extract32(encoding, 29, 3));
+    return iclass;
+}
+
+/*
+ * Per table 10-5, the duplex ICLASS field values that specify the group of
+ * each sub-instruction in a duplex
+ *
+ * This table points to the decode instruction for each entry in the table
+ */
+typedef bool (*subinsn_decode_func)(DisasContext *ctx, uint16_t insn);
+typedef struct {
+    subinsn_decode_func decode_slot0_subinsn;
+    subinsn_decode_func decode_slot1_subinsn;
+} subinsn_decode_groups;
+
+static const subinsn_decode_groups decode_groups[16] = {
+    [0x0] = { decode_subinsn_l1, decode_subinsn_l1 },
+    [0x1] = { decode_subinsn_l2, decode_subinsn_l1 },
+    [0x2] = { decode_subinsn_l2, decode_subinsn_l2 },
+    [0x3] = { decode_subinsn_a,  decode_subinsn_a },
+    [0x4] = { decode_subinsn_l1, decode_subinsn_a },
+    [0x5] = { decode_subinsn_l2, decode_subinsn_a },
+    [0x6] = { decode_subinsn_s1, decode_subinsn_a },
+    [0x7] = { decode_subinsn_s2, decode_subinsn_a },
+    [0x8] = { decode_subinsn_s1, decode_subinsn_l1 },
+    [0x9] = { decode_subinsn_s1, decode_subinsn_l2 },
+    [0xa] = { decode_subinsn_s1, decode_subinsn_s1 },
+    [0xb] = { decode_subinsn_s2, decode_subinsn_s1 },
+    [0xc] = { decode_subinsn_s2, decode_subinsn_l1 },
+    [0xd] = { decode_subinsn_s2, decode_subinsn_l2 },
+    [0xe] = { decode_subinsn_s2, decode_subinsn_s2 },
+    [0xf] = { NULL,              NULL },              /* Reserved */
+};
+
+static uint16_t get_slot0_subinsn(uint32_t encoding)
+{
+    return extract32(encoding, 0, 13);
+}
+
+static uint16_t get_slot1_subinsn(uint32_t encoding)
+{
+    return extract32(encoding, 16, 13);
+}
+
 static unsigned int
 decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
 {
@@ -805,8 +870,28 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
         table = &dectree_table_DECODE_ROOT_32;
         g_assert_not_reached();
     } else {
+        uint32_t iclass = get_duplex_iclass(encoding);
+        unsigned int slot0_subinsn = get_slot0_subinsn(encoding);
+        unsigned int slot1_subinsn = get_slot1_subinsn(encoding);
+        subinsn_decode_func decode_slot0_subinsn =
+            decode_groups[iclass].decode_slot0_subinsn;
+        subinsn_decode_func decode_slot1_subinsn =
+            decode_groups[iclass].decode_slot1_subinsn;
+
+        /* The slot1 subinsn needs to be in the packet first */
+        if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
+            insn->generate = opcode_genptr[insn->opcode];
+            insn->iclass = iclass_bits(encoding);
+            ctx->insn = ++insn;
+            if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
+                insn->generate = opcode_genptr[insn->opcode];
+                insn->iclass = iclass_bits(encoding);
+                return 2;
+            }
+        }
         /* start with EE table - duplex instructions */
         table = &dectree_table_DECODE_ROOT_EE;
+        g_assert_not_reached();
     }
     return decode_insns_tablewalk(insn, table, encoding);
 }
diff --git a/target/hexagon/README b/target/hexagon/README
index 1b2a4d0eac..746ebec378 100644
--- a/target/hexagon/README
+++ b/target/hexagon/README
@@ -195,6 +195,7 @@ Step 1 is to run target/hexagon/gen_dectree_import.c to produce
 Step 2 is to import iset.py into target/hexagon/gen_decodetree.py to produce
     <BUILD_DIR>/target/hexagon/normal_decode_generated
     <BUILD_DIR>/target/hexagon/hvx_decode_generated
+    <BUILD_DIR>/target/hexagon/subinsn_*_decode_generated
 Step 3 is to process the above files with QEMU's decodetree.py to produce
     <BUILD_DIR>/target/hexagon/decode_*_generated.c.inc
 Step 4 is to import iset.py into target/hexagon/gen_trans_funcs.py to produce
diff --git a/target/hexagon/gen_decodetree.py b/target/hexagon/gen_decodetree.py
index 1693148ec0..2dff975f55 100755
--- a/target/hexagon/gen_decodetree.py
+++ b/target/hexagon/gen_decodetree.py
@@ -96,8 +96,10 @@ def skip_tag(tag, class_to_decode):
 ##     A2_add   ..................-.....---..... @A2_add
 ##
 def gen_decodetree_file(f, class_to_decode):
+    is_subinsn = class_to_decode.startswith("SUBINSN_")
     f.write(f"## DO NOT MODIFY - This file is generated by {sys.argv[0]}\n\n")
-    f.write("%PP\t14:2\n\n")
+    if not is_subinsn:
+        f.write("%PP\t14:2\n\n")
     for tag in sorted(encs.keys(), key=iset.tags.index):
         if skip_tag(tag, class_to_decode):
             continue
@@ -107,6 +109,12 @@ def gen_decodetree_file(f, class_to_decode):
 
         enc = encs[tag]
         enc_str = "".join(reversed(encs[tag]))
+
+        # The subinstructions come with a 13-bit encoding, but
+        # decodetree.py needs 16 bits
+        if is_subinsn:
+            enc_str = "---" + enc_str
+
         f.write(f"## {tag}:\t{enc_str}\n")
         f.write("##\n")
 
@@ -176,7 +184,9 @@ def gen_decodetree_file(f, class_to_decode):
             imm_letter = "i" if imm_type.islower() else "I"
             f.write(f" {imm_type}{imm_letter}=%{tag}_{imm_type}{imm_letter}")
 
-        f.write(" %PP\n")
+        if not is_subinsn:
+            f.write(" %PP")
+        f.write("\n")
 
          # Replace the 0s and 1s with .
         for x in { "0", "1" }:
diff --git a/target/hexagon/gen_trans_funcs.py b/target/hexagon/gen_trans_funcs.py
index c907131009..53e844a44b 100755
--- a/target/hexagon/gen_trans_funcs.py
+++ b/target/hexagon/gen_trans_funcs.py
@@ -40,11 +40,6 @@ def ordered_unique(l):
     return sorted(set(l), key=l.index)
 
 
-def skip_tag(tag, classes):
-    enc_class = iset.iset[tag]["enc_class"]
-    return enc_class not in classes
-
-
 def code_fmt(txt):
     return textwrap.indent(textwrap.dedent(txt), "    ")
 
@@ -76,12 +71,9 @@ def mark_which_imm_extended(f, tag):
 ##         return true;
 ##     }
 ##
-def gen_trans_funcs(f, classes):
+def gen_trans_funcs(f):
     f.write(f"/* DO NOT MODIFY - This file is generated by {sys.argv[0]} */\n\n")
     for tag in sorted(encs.keys(), key=iset.tags.index):
-        if skip_tag(tag, classes):
-            continue
-
         regs = ordered_unique(regre.findall(iset.iset[tag]["syntax"]))
         imms = ordered_unique(immre.findall(iset.iset[tag]["syntax"]))
 
@@ -129,4 +121,4 @@ def gen_trans_funcs(f, classes):
 if __name__ == "__main__":
     hex_common.read_semantics_file(sys.argv[1])
     with open(sys.argv[2], "w") as f:
-        gen_trans_funcs(f, { "NORMAL", "EXT_mmvec" })
+        gen_trans_funcs(f)
diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index 831bd5716a..1bd1ebcba1 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -154,6 +154,51 @@ hvx_decode_generated = custom_target(
 )
 hexagon_ss.add(hvx_decode_generated)
 
+subinsn_a_decode_generated = custom_target(
+    'subinsn_a_decode_generated',
+    output: 'subinsn_a_decode_generated',
+    depends: [iset_py, semantics_generated],
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files('gen_decodetree.py'), semantics_generated, 'SUBINSN_A', '@OUTPUT@'],
+)
+hexagon_ss.add(subinsn_a_decode_generated)
+
+subinsn_l1_decode_generated = custom_target(
+    'subinsn_l1_decode_generated',
+    output: 'subinsn_l1_decode_generated',
+    depends: [iset_py, semantics_generated],
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files('gen_decodetree.py'), semantics_generated, 'SUBINSN_L1', '@OUTPUT@'],
+)
+hexagon_ss.add(subinsn_l1_decode_generated)
+
+subinsn_l2_decode_generated = custom_target(
+    'subinsn_l2_decode_generated',
+    output: 'subinsn_l2_decode_generated',
+    depends: [iset_py, semantics_generated],
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files('gen_decodetree.py'), semantics_generated, 'SUBINSN_L2', '@OUTPUT@'],
+)
+hexagon_ss.add(subinsn_l2_decode_generated)
+
+subinsn_s1_decode_generated = custom_target(
+    'subinsn_s1_decode_generated',
+    output: 'subinsn_s1_decode_generated',
+    depends: [iset_py, semantics_generated],
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files('gen_decodetree.py'), semantics_generated, 'SUBINSN_S1', '@OUTPUT@'],
+)
+hexagon_ss.add(subinsn_s1_decode_generated)
+
+subinsn_s2_decode_generated = custom_target(
+    'subinsn_s2_decode_generated',
+    output: 'subinsn_s2_decode_generated',
+    depends: [iset_py, semantics_generated],
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files('gen_decodetree.py'), semantics_generated, 'SUBINSN_S2', '@OUTPUT@'],
+)
+hexagon_ss.add(subinsn_s2_decode_generated)
+
 #
 # Run the QEMU decodetree.py script to produce the instruction decoder
 #
@@ -176,6 +221,51 @@ decode_hvx_generated = custom_target(
 )
 hexagon_ss.add(decode_hvx_generated)
 
+decode_subinsn_a_generated = custom_target(
+    'decode_subinsn_a_generated.c.inc',
+    output: 'decode_subinsn_a_generated.c.inc',
+    input: subinsn_a_decode_generated,
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files(decodetree_py), subinsn_a_decode_generated, ['--static-decode=decode_subinsn_a', '--insnwidth=16'], '-o', '@OUTPUT@'],
+)
+hexagon_ss.add(decode_subinsn_a_generated)
+
+decode_subinsn_l1_generated = custom_target(
+    'decode_subinsn_l1_generated.c.inc',
+    output: 'decode_subinsn_l1_generated.c.inc',
+    input: subinsn_l1_decode_generated,
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files(decodetree_py), subinsn_l1_decode_generated, ['--static-decode=decode_subinsn_l1', '--insnwidth=16'], '-o', '@OUTPUT@'],
+)
+hexagon_ss.add(decode_subinsn_l1_generated)
+
+decode_subinsn_l2_generated = custom_target(
+    'decode_subinsn_l2_generated.c.inc',
+    output: 'decode_subinsn_l2_generated.c.inc',
+    input: subinsn_l2_decode_generated,
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files(decodetree_py), subinsn_l2_decode_generated, ['--static-decode=decode_subinsn_l2', '--insnwidth=16'], '-o', '@OUTPUT@'],
+)
+hexagon_ss.add(decode_subinsn_l2_generated)
+
+decode_subinsn_s1_generated = custom_target(
+    'decode_subinsn_s1_generated.c.inc',
+    output: 'decode_subinsn_s1_generated.c.inc',
+    input: subinsn_s1_decode_generated,
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files(decodetree_py), subinsn_s1_decode_generated, ['--static-decode=decode_subinsn_s1', '--insnwidth=16'], '-o', '@OUTPUT@'],
+)
+hexagon_ss.add(decode_subinsn_s1_generated)
+
+decode_subinsn_s2_generated = custom_target(
+    'decode_subinsn_s2_generated.c.inc',
+    output: 'decode_subinsn_s2_generated.c.inc',
+    input: subinsn_s2_decode_generated,
+    env: {'PYTHONPATH': meson.current_build_dir()},
+    command: [python, files(decodetree_py), subinsn_s2_decode_generated, ['--static-decode=decode_subinsn_s2', '--insnwidth=16'], '-o', '@OUTPUT@'],
+)
+hexagon_ss.add(decode_subinsn_s2_generated)
+
 #
 # Generate the trans_* functions that the decoder will use
 #
-- 
2.34.1
RE: [PATCH v2 2/3] Hexagon (target/hexagon) Use QEMU decodetree (16-bit instructions)
Posted by Brian Cain 10 months, 2 weeks ago

> -----Original Message-----
> From: Taylor Simpson <ltaylorsimpson@gmail.com>
> Sent: Monday, January 8, 2024 4:49 PM
> To: qemu-devel@nongnu.org
> Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Marco
> Liebel (QUIC) <quic_mliebel@quicinc.com>; richard.henderson@linaro.org;
> philmd@linaro.org; ale@rev.ng; anjo@rev.ng; ltaylorsimpson@gmail.com
> Subject: [PATCH v2 2/3] Hexagon (target/hexagon) Use QEMU decodetree (16-
> bit instructions)
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
> 
> Section 10.3 of the Hexagon V73 Programmer's Reference Manual
> 
> A duplex is encoded as a 32-bit instruction with bits [15:14] set to 00.
> The sub-instructions that comprise a duplex are encoded as 13-bit fields
> in the duplex.
> 
> Create a decoder for each subinstruction class (a, l1, l2, s1, s2).
> 
> Extend gen_trans_funcs.py to handle all instructions rather than
> filter by instruction class.
> 
> There is a g_assert_not_reached() in decode_insns() in decode.c to
> verify we never try to use the old decoder on 16-bit instructions.
> 
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
>  target/hexagon/decode.c           | 85 +++++++++++++++++++++++++++++
>  target/hexagon/README             |  1 +
>  target/hexagon/gen_decodetree.py  | 14 ++++-
>  target/hexagon/gen_trans_funcs.py | 12 +----
>  target/hexagon/meson.build        | 90 +++++++++++++++++++++++++++++++
>  5 files changed, 190 insertions(+), 12 deletions(-)
> 
> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
> index bddad1f75e..160b23a895 100644
> --- a/target/hexagon/decode.c
> +++ b/target/hexagon/decode.c
> @@ -60,6 +60,7 @@ static int decode_mapped_reg_##NAME(DisasContext
> *ctx, int x) \
>  }
>  DECODE_MAPPED(R_16)
>  DECODE_MAPPED(R_8)
> +DECODE_MAPPED(R__8)
> 
>  /* Helper function for decodetree_trans_funcs_generated.c.inc */
>  static int shift_left(DisasContext *ctx, int x, int n, int immno)
> @@ -77,6 +78,13 @@ static int shift_left(DisasContext *ctx, int x, int n, int
> immno)
>  #include "decode_normal_generated.c.inc"
>  #include "decode_hvx_generated.c.inc"
> 
> +/* Include the generated decoder for 16 bit insn */
> +#include "decode_subinsn_a_generated.c.inc"
> +#include "decode_subinsn_l1_generated.c.inc"
> +#include "decode_subinsn_l2_generated.c.inc"
> +#include "decode_subinsn_s1_generated.c.inc"
> +#include "decode_subinsn_s2_generated.c.inc"
> +
>  /* Include the generated helpers for the decoder */
>  #include "decodetree_trans_funcs_generated.c.inc"
> 
> @@ -790,6 +798,63 @@ decode_insns_tablewalk(Insn *insn, const
> DectreeTable *table,
>      }
>  }
> 
> +/*
> + * Section 10.3 of the Hexagon V73 Programmer's Reference Manual
> + *
> + * A duplex is encoded as a 32-bit instruction with bits [15:14] set to 00.
> + * The sub-instructions that comprise a duplex are encoded as 13-bit fields
> + * in the duplex.
> + *
> + * Per table 10-4, the 4-bit duplex iclass is encoded in bits 31:29, 13
> + */
> +static uint32_t get_duplex_iclass(uint32_t encoding)
> +{
> +    uint32_t iclass = extract32(encoding, 13, 1);
> +    iclass = deposit32(iclass, 1, 3, extract32(encoding, 29, 3));
> +    return iclass;
> +}
> +
> +/*
> + * Per table 10-5, the duplex ICLASS field values that specify the group of
> + * each sub-instruction in a duplex
> + *
> + * This table points to the decode instruction for each entry in the table
> + */
> +typedef bool (*subinsn_decode_func)(DisasContext *ctx, uint16_t insn);
> +typedef struct {
> +    subinsn_decode_func decode_slot0_subinsn;
> +    subinsn_decode_func decode_slot1_subinsn;
> +} subinsn_decode_groups;
> +
> +static const subinsn_decode_groups decode_groups[16] = {
> +    [0x0] = { decode_subinsn_l1, decode_subinsn_l1 },
> +    [0x1] = { decode_subinsn_l2, decode_subinsn_l1 },
> +    [0x2] = { decode_subinsn_l2, decode_subinsn_l2 },
> +    [0x3] = { decode_subinsn_a,  decode_subinsn_a },
> +    [0x4] = { decode_subinsn_l1, decode_subinsn_a },
> +    [0x5] = { decode_subinsn_l2, decode_subinsn_a },
> +    [0x6] = { decode_subinsn_s1, decode_subinsn_a },
> +    [0x7] = { decode_subinsn_s2, decode_subinsn_a },
> +    [0x8] = { decode_subinsn_s1, decode_subinsn_l1 },
> +    [0x9] = { decode_subinsn_s1, decode_subinsn_l2 },
> +    [0xa] = { decode_subinsn_s1, decode_subinsn_s1 },
> +    [0xb] = { decode_subinsn_s2, decode_subinsn_s1 },
> +    [0xc] = { decode_subinsn_s2, decode_subinsn_l1 },
> +    [0xd] = { decode_subinsn_s2, decode_subinsn_l2 },
> +    [0xe] = { decode_subinsn_s2, decode_subinsn_s2 },
> +    [0xf] = { NULL,              NULL },              /* Reserved */
> +};
> +
> +static uint16_t get_slot0_subinsn(uint32_t encoding)
> +{
> +    return extract32(encoding, 0, 13);
> +}
> +
> +static uint16_t get_slot1_subinsn(uint32_t encoding)
> +{
> +    return extract32(encoding, 16, 13);
> +}
> +
>  static unsigned int
>  decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
>  {
> @@ -805,8 +870,28 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t
> encoding)
>          table = &dectree_table_DECODE_ROOT_32;
>          g_assert_not_reached();
>      } else {
> +        uint32_t iclass = get_duplex_iclass(encoding);
> +        unsigned int slot0_subinsn = get_slot0_subinsn(encoding);
> +        unsigned int slot1_subinsn = get_slot1_subinsn(encoding);
> +        subinsn_decode_func decode_slot0_subinsn =
> +            decode_groups[iclass].decode_slot0_subinsn;
> +        subinsn_decode_func decode_slot1_subinsn =
> +            decode_groups[iclass].decode_slot1_subinsn;
> +
> +        /* The slot1 subinsn needs to be in the packet first */
> +        if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
> +            insn->generate = opcode_genptr[insn->opcode];
> +            insn->iclass = iclass_bits(encoding);
> +            ctx->insn = ++insn;
> +            if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
> +                insn->generate = opcode_genptr[insn->opcode];
> +                insn->iclass = iclass_bits(encoding);
> +                return 2;
> +            }
> +        }
>          /* start with EE table - duplex instructions */
>          table = &dectree_table_DECODE_ROOT_EE;
> +        g_assert_not_reached();
>      }
>      return decode_insns_tablewalk(insn, table, encoding);
>  }
> diff --git a/target/hexagon/README b/target/hexagon/README
> index 1b2a4d0eac..746ebec378 100644
> --- a/target/hexagon/README
> +++ b/target/hexagon/README
> @@ -195,6 +195,7 @@ Step 1 is to run target/hexagon/gen_dectree_import.c
> to produce
>  Step 2 is to import iset.py into target/hexagon/gen_decodetree.py to produce
>      <BUILD_DIR>/target/hexagon/normal_decode_generated
>      <BUILD_DIR>/target/hexagon/hvx_decode_generated
> +    <BUILD_DIR>/target/hexagon/subinsn_*_decode_generated
>  Step 3 is to process the above files with QEMU's decodetree.py to produce
>      <BUILD_DIR>/target/hexagon/decode_*_generated.c.inc
>  Step 4 is to import iset.py into target/hexagon/gen_trans_funcs.py to produce
> diff --git a/target/hexagon/gen_decodetree.py
> b/target/hexagon/gen_decodetree.py
> index 1693148ec0..2dff975f55 100755
> --- a/target/hexagon/gen_decodetree.py
> +++ b/target/hexagon/gen_decodetree.py
> @@ -96,8 +96,10 @@ def skip_tag(tag, class_to_decode):
>  ##     A2_add   ..................-.....---..... @A2_add
>  ##
>  def gen_decodetree_file(f, class_to_decode):
> +    is_subinsn = class_to_decode.startswith("SUBINSN_")
>      f.write(f"## DO NOT MODIFY - This file is generated by {sys.argv[0]}\n\n")
> -    f.write("%PP\t14:2\n\n")
> +    if not is_subinsn:
> +        f.write("%PP\t14:2\n\n")
>      for tag in sorted(encs.keys(), key=iset.tags.index):
>          if skip_tag(tag, class_to_decode):
>              continue
> @@ -107,6 +109,12 @@ def gen_decodetree_file(f, class_to_decode):
> 
>          enc = encs[tag]
>          enc_str = "".join(reversed(encs[tag]))
> +
> +        # The subinstructions come with a 13-bit encoding, but
> +        # decodetree.py needs 16 bits
> +        if is_subinsn:
> +            enc_str = "---" + enc_str
> +
>          f.write(f"## {tag}:\t{enc_str}\n")
>          f.write("##\n")
> 
> @@ -176,7 +184,9 @@ def gen_decodetree_file(f, class_to_decode):
>              imm_letter = "i" if imm_type.islower() else "I"
>              f.write(f" {imm_type}{imm_letter}=%{tag}_{imm_type}{imm_letter}")
> 
> -        f.write(" %PP\n")
> +        if not is_subinsn:
> +            f.write(" %PP")
> +        f.write("\n")
> 
>           # Replace the 0s and 1s with .
>          for x in { "0", "1" }:
> diff --git a/target/hexagon/gen_trans_funcs.py
> b/target/hexagon/gen_trans_funcs.py
> index c907131009..53e844a44b 100755
> --- a/target/hexagon/gen_trans_funcs.py
> +++ b/target/hexagon/gen_trans_funcs.py
> @@ -40,11 +40,6 @@ def ordered_unique(l):
>      return sorted(set(l), key=l.index)
> 
> 
> -def skip_tag(tag, classes):
> -    enc_class = iset.iset[tag]["enc_class"]
> -    return enc_class not in classes
> -
> -
>  def code_fmt(txt):
>      return textwrap.indent(textwrap.dedent(txt), "    ")
> 
> @@ -76,12 +71,9 @@ def mark_which_imm_extended(f, tag):
>  ##         return true;
>  ##     }
>  ##
> -def gen_trans_funcs(f, classes):
> +def gen_trans_funcs(f):
>      f.write(f"/* DO NOT MODIFY - This file is generated by {sys.argv[0]} */\n\n")
>      for tag in sorted(encs.keys(), key=iset.tags.index):
> -        if skip_tag(tag, classes):
> -            continue
> -
>          regs = ordered_unique(regre.findall(iset.iset[tag]["syntax"]))
>          imms = ordered_unique(immre.findall(iset.iset[tag]["syntax"]))
> 
> @@ -129,4 +121,4 @@ def gen_trans_funcs(f, classes):
>  if __name__ == "__main__":
>      hex_common.read_semantics_file(sys.argv[1])
>      with open(sys.argv[2], "w") as f:
> -        gen_trans_funcs(f, { "NORMAL", "EXT_mmvec" })
> +        gen_trans_funcs(f)
> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> index 831bd5716a..1bd1ebcba1 100644
> --- a/target/hexagon/meson.build
> +++ b/target/hexagon/meson.build
> @@ -154,6 +154,51 @@ hvx_decode_generated = custom_target(
>  )
>  hexagon_ss.add(hvx_decode_generated)
> 
> +subinsn_a_decode_generated = custom_target(
> +    'subinsn_a_decode_generated',
> +    output: 'subinsn_a_decode_generated',
> +    depends: [iset_py, semantics_generated],
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files('gen_decodetree.py'), semantics_generated,
> 'SUBINSN_A', '@OUTPUT@'],
> +)
> +hexagon_ss.add(subinsn_a_decode_generated)
> +
> +subinsn_l1_decode_generated = custom_target(
> +    'subinsn_l1_decode_generated',
> +    output: 'subinsn_l1_decode_generated',
> +    depends: [iset_py, semantics_generated],
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files('gen_decodetree.py'), semantics_generated,
> 'SUBINSN_L1', '@OUTPUT@'],
> +)
> +hexagon_ss.add(subinsn_l1_decode_generated)
> +
> +subinsn_l2_decode_generated = custom_target(
> +    'subinsn_l2_decode_generated',
> +    output: 'subinsn_l2_decode_generated',
> +    depends: [iset_py, semantics_generated],
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files('gen_decodetree.py'), semantics_generated,
> 'SUBINSN_L2', '@OUTPUT@'],
> +)
> +hexagon_ss.add(subinsn_l2_decode_generated)
> +
> +subinsn_s1_decode_generated = custom_target(
> +    'subinsn_s1_decode_generated',
> +    output: 'subinsn_s1_decode_generated',
> +    depends: [iset_py, semantics_generated],
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files('gen_decodetree.py'), semantics_generated,
> 'SUBINSN_S1', '@OUTPUT@'],
> +)
> +hexagon_ss.add(subinsn_s1_decode_generated)
> +
> +subinsn_s2_decode_generated = custom_target(
> +    'subinsn_s2_decode_generated',
> +    output: 'subinsn_s2_decode_generated',
> +    depends: [iset_py, semantics_generated],
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files('gen_decodetree.py'), semantics_generated,
> 'SUBINSN_S2', '@OUTPUT@'],
> +)
> +hexagon_ss.add(subinsn_s2_decode_generated)
> +
>  #
>  # Run the QEMU decodetree.py script to produce the instruction decoder
>  #
> @@ -176,6 +221,51 @@ decode_hvx_generated = custom_target(
>  )
>  hexagon_ss.add(decode_hvx_generated)
> 
> +decode_subinsn_a_generated = custom_target(
> +    'decode_subinsn_a_generated.c.inc',
> +    output: 'decode_subinsn_a_generated.c.inc',
> +    input: subinsn_a_decode_generated,
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files(decodetree_py), subinsn_a_decode_generated, ['--
> static-decode=decode_subinsn_a', '--insnwidth=16'], '-o', '@OUTPUT@'],
> +)
> +hexagon_ss.add(decode_subinsn_a_generated)
> +
> +decode_subinsn_l1_generated = custom_target(
> +    'decode_subinsn_l1_generated.c.inc',
> +    output: 'decode_subinsn_l1_generated.c.inc',
> +    input: subinsn_l1_decode_generated,
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files(decodetree_py), subinsn_l1_decode_generated, ['--
> static-decode=decode_subinsn_l1', '--insnwidth=16'], '-o', '@OUTPUT@'],
> +)
> +hexagon_ss.add(decode_subinsn_l1_generated)
> +
> +decode_subinsn_l2_generated = custom_target(
> +    'decode_subinsn_l2_generated.c.inc',
> +    output: 'decode_subinsn_l2_generated.c.inc',
> +    input: subinsn_l2_decode_generated,
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files(decodetree_py), subinsn_l2_decode_generated, ['--
> static-decode=decode_subinsn_l2', '--insnwidth=16'], '-o', '@OUTPUT@'],
> +)
> +hexagon_ss.add(decode_subinsn_l2_generated)
> +
> +decode_subinsn_s1_generated = custom_target(
> +    'decode_subinsn_s1_generated.c.inc',
> +    output: 'decode_subinsn_s1_generated.c.inc',
> +    input: subinsn_s1_decode_generated,
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files(decodetree_py), subinsn_s1_decode_generated, ['-
> -static-decode=decode_subinsn_s1', '--insnwidth=16'], '-o', '@OUTPUT@'],
> +)
> +hexagon_ss.add(decode_subinsn_s1_generated)
> +
> +decode_subinsn_s2_generated = custom_target(
> +    'decode_subinsn_s2_generated.c.inc',
> +    output: 'decode_subinsn_s2_generated.c.inc',
> +    input: subinsn_s2_decode_generated,
> +    env: {'PYTHONPATH': meson.current_build_dir()},
> +    command: [python, files(decodetree_py), subinsn_s2_decode_generated, ['-
> -static-decode=decode_subinsn_s2', '--insnwidth=16'], '-o', '@OUTPUT@'],
> +)
> +hexagon_ss.add(decode_subinsn_s2_generated)
> +
>  #
>  # Generate the trans_* functions that the decoder will use
>  #
> --
> 2.34.1

Reviewed-by: Brian Cain <bcain@quicinc.com>