diff mbox series

[PULL,v2,25/30] Hexagon HVX (target/hexagon) instruction decoding

Message ID 1635974247-1820-26-git-send-email-tsimpson@quicinc.com
State New
Headers show
Series [PULL,v2,01/30] Hexagon HVX (target/hexagon) README | expand

Commit Message

Taylor Simpson Nov. 3, 2021, 9:17 p.m. UTC
Add new file to target/hexagon/meson.build

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/mmvec/decode_ext_mmvec.h |  24 ++++
 target/hexagon/decode.c                 |  24 +++-
 target/hexagon/mmvec/decode_ext_mmvec.c | 236 ++++++++++++++++++++++++++++++++
 target/hexagon/meson.build              |   1 +
 4 files changed, 283 insertions(+), 2 deletions(-)
 create mode 100644 target/hexagon/mmvec/decode_ext_mmvec.h
 create mode 100644 target/hexagon/mmvec/decode_ext_mmvec.c

Comments

Peter Maydell Nov. 21, 2023, 2:33 p.m. UTC | #1
On Wed, 3 Nov 2021 at 21:17, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> Add new file to target/hexagon/meson.build
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>

Hi; Coverity points out a variable written to and then
overwritten before it's ever used in this function (CID 1527408):




> +static void
> +check_new_value(Packet *pkt)
> +{
> +    /* .new value for a MMVector store */
> +    int i, j;
> +    const char *reginfo;
> +    const char *destletters;
> +    const char *dststr = NULL;
> +    uint16_t def_opcode;
> +    char letter;
> +    int def_regnum;

def_regnum has function level scope...

> +
> +    for (i = 1; i < pkt->num_insns; i++) {
> +        uint16_t use_opcode = pkt->insn[i].opcode;
> +        if (GET_ATTRIB(use_opcode, A_DOTNEWVALUE) &&
> +            GET_ATTRIB(use_opcode, A_CVI) &&
> +            GET_ATTRIB(use_opcode, A_STORE)) {
> +            int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
> +                opcode_reginfo[use_opcode];
> +            /*
> +             * What's encoded at the N-field is the offset to who's producing
> +             * the value.
> +             * Shift off the LSB which indicates odd/even register.
> +             */
> +            int def_off = ((pkt->insn[i].regno[use_regidx]) >> 1);
> +            int def_oreg = pkt->insn[i].regno[use_regidx] & 1;
> +            int def_idx = -1;
> +            for (j = i - 1; (j >= 0) && (def_off >= 0); j--) {
> +                if (!GET_ATTRIB(pkt->insn[j].opcode, A_CVI)) {
> +                    continue;
> +                }
> +                def_off--;
> +                if (def_off == 0) {
> +                    def_idx = j;
> +                    break;
> +                }
> +            }
> +            /*
> +             * Check for a badly encoded N-field which points to an instruction
> +             * out-of-range
> +             */
> +            g_assert(!((def_off != 0) || (def_idx < 0) ||
> +                       (def_idx > (pkt->num_insns - 1))));
> +
> +            /* def_idx is the index of the producer */
> +            def_opcode = pkt->insn[def_idx].opcode;
> +            reginfo = opcode_reginfo[def_opcode];
> +            destletters = "dexy";
> +            for (j = 0; (letter = destletters[j]) != 0; j++) {
> +                dststr = strchr(reginfo, letter);
> +                if (dststr != NULL) {
> +                    break;
> +                }
> +            }
> +            if ((dststr == NULL)  && GET_ATTRIB(def_opcode, A_CVI_GATHER)) {
> +                def_regnum = 0;

In this half of the if() we set it to 0...

> +                pkt->insn[i].regno[use_regidx] = def_oreg;
> +                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
> +            } else {
> +                if (dststr == NULL) {
> +                    /* still not there, we have a bad packet */
> +                    g_assert_not_reached();
> +                }
> +                def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> +                /* Now patch up the consumer with the register number */
> +                pkt->insn[i].regno[use_regidx] = def_regnum ^ def_oreg;
> +                /* special case for (Vx,Vy) */
> +                dststr = strchr(reginfo, 'y');
> +                if (def_oreg && strchr(reginfo, 'x') && dststr) {
> +                    def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> +                    pkt->insn[i].regno[use_regidx] = def_regnum;
> +                }

...but the only place we read def_regnum is in this other half of the
if(), and if we get here then we've set it to something out of pxt->insn.

Were we supposed to do something with def_regnum outside this if(),
or could we instead drop the initialization in the first half of the if()
and move its declaration inside this else {} block ?

> +                /*
> +                 * We need to remember who produces this value to later
> +                 * check if it was dynamically cancelled
> +                 */
> +                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
> +            }
> +        }
> +    }
> +}

thanks
-- PMM
Brian Cain Nov. 21, 2023, 3:51 p.m. UTC | #2
> -----Original Message-----
> From: qemu-devel-bounces+bcain=quicinc.com@nongnu.org <qemu-devel-
> bounces+bcain=quicinc.com@nongnu.org> On Behalf Of Peter Maydell
> Sent: Tuesday, November 21, 2023 8:33 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel@nongnu.org; richard.henderson@linaro.org; f4bug@amsat.org
> Subject: Re: [PULL v2 25/30] Hexagon HVX (target/hexagon) instruction
> decoding
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
> 
> On Wed, 3 Nov 2021 at 21:17, Taylor Simpson <tsimpson@quicinc.com> wrote:
> >
> > Add new file to target/hexagon/meson.build
> >
> > Acked-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> 
> Hi; Coverity points out a variable written to and then
> overwritten before it's ever used in this function (CID 1527408):
> 
> 
> 
> 
> > +static void
> > +check_new_value(Packet *pkt)
> > +{
> > +    /* .new value for a MMVector store */
> > +    int i, j;
> > +    const char *reginfo;
> > +    const char *destletters;
> > +    const char *dststr = NULL;
> > +    uint16_t def_opcode;
> > +    char letter;
> > +    int def_regnum;
> 
> def_regnum has function level scope...
> 
> > +
> > +    for (i = 1; i < pkt->num_insns; i++) {
> > +        uint16_t use_opcode = pkt->insn[i].opcode;
> > +        if (GET_ATTRIB(use_opcode, A_DOTNEWVALUE) &&
> > +            GET_ATTRIB(use_opcode, A_CVI) &&
> > +            GET_ATTRIB(use_opcode, A_STORE)) {
> > +            int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
> > +                opcode_reginfo[use_opcode];
> > +            /*
> > +             * What's encoded at the N-field is the offset to who's producing
> > +             * the value.
> > +             * Shift off the LSB which indicates odd/even register.
> > +             */
> > +            int def_off = ((pkt->insn[i].regno[use_regidx]) >> 1);
> > +            int def_oreg = pkt->insn[i].regno[use_regidx] & 1;
> > +            int def_idx = -1;
> > +            for (j = i - 1; (j >= 0) && (def_off >= 0); j--) {
> > +                if (!GET_ATTRIB(pkt->insn[j].opcode, A_CVI)) {
> > +                    continue;
> > +                }
> > +                def_off--;
> > +                if (def_off == 0) {
> > +                    def_idx = j;
> > +                    break;
> > +                }
> > +            }
> > +            /*
> > +             * Check for a badly encoded N-field which points to an instruction
> > +             * out-of-range
> > +             */
> > +            g_assert(!((def_off != 0) || (def_idx < 0) ||
> > +                       (def_idx > (pkt->num_insns - 1))));
> > +
> > +            /* def_idx is the index of the producer */
> > +            def_opcode = pkt->insn[def_idx].opcode;
> > +            reginfo = opcode_reginfo[def_opcode];
> > +            destletters = "dexy";
> > +            for (j = 0; (letter = destletters[j]) != 0; j++) {
> > +                dststr = strchr(reginfo, letter);
> > +                if (dststr != NULL) {
> > +                    break;
> > +                }
> > +            }
> > +            if ((dststr == NULL)  && GET_ATTRIB(def_opcode, A_CVI_GATHER)) {
> > +                def_regnum = 0;
> 
> In this half of the if() we set it to 0...
> 
> > +                pkt->insn[i].regno[use_regidx] = def_oreg;
> > +                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
> > +            } else {
> > +                if (dststr == NULL) {
> > +                    /* still not there, we have a bad packet */
> > +                    g_assert_not_reached();
> > +                }
> > +                def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> > +                /* Now patch up the consumer with the register number */
> > +                pkt->insn[i].regno[use_regidx] = def_regnum ^ def_oreg;
> > +                /* special case for (Vx,Vy) */
> > +                dststr = strchr(reginfo, 'y');
> > +                if (def_oreg && strchr(reginfo, 'x') && dststr) {
> > +                    def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> > +                    pkt->insn[i].regno[use_regidx] = def_regnum;
> > +                }
> 
> ...but the only place we read def_regnum is in this other half of the
> if(), and if we get here then we've set it to something out of pxt->insn.
> 
> Were we supposed to do something with def_regnum outside this if(),
> or could we instead drop the initialization in the first half of the if()
> and move its declaration inside this else {} block ?

Hmm -- we'll take a look at this and get back to you.

> > +                /*
> > +                 * We need to remember who produces this value to later
> > +                 * check if it was dynamically cancelled
> > +                 */
> > +                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
> > +            }
> > +        }
> > +    }
> > +}
> 
> thanks
> -- PMM
Brian Cain Nov. 27, 2023, 4:15 a.m. UTC | #3
> -----Original Message-----
> From: Brian Cain
> Sent: Tuesday, November 21, 2023 9:52 AM
> To: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-devel@nongnu.org; richard.henderson@linaro.org; f4bug@amsat.org
> Subject: RE: [PULL v2 25/30] Hexagon HVX (target/hexagon) instruction
> decoding
> 
> 
> 
> > -----Original Message-----
> > From: qemu-devel-bounces+bcain=quicinc.com@nongnu.org <qemu-devel-
> > bounces+bcain=quicinc.com@nongnu.org> On Behalf Of Peter Maydell
> > Sent: Tuesday, November 21, 2023 8:33 AM
> > To: Taylor Simpson <tsimpson@quicinc.com>
> > Cc: qemu-devel@nongnu.org; richard.henderson@linaro.org;
> f4bug@amsat.org
> > Subject: Re: [PULL v2 25/30] Hexagon HVX (target/hexagon) instruction
> > decoding
> >
> > WARNING: This email originated from outside of Qualcomm. Please be wary
> of
> > any links or attachments, and do not enable macros.
> >
> > On Wed, 3 Nov 2021 at 21:17, Taylor Simpson <tsimpson@quicinc.com>
> wrote:
> > >
> > > Add new file to target/hexagon/meson.build
> > >
> > > Acked-by: Richard Henderson <richard.henderson@linaro.org>
> > > Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> >
> > Hi; Coverity points out a variable written to and then
> > overwritten before it's ever used in this function (CID 1527408):
> >
> >
> >
> >
> > > +static void
> > > +check_new_value(Packet *pkt)
> > > +{
> > > +    /* .new value for a MMVector store */
> > > +    int i, j;
> > > +    const char *reginfo;
> > > +    const char *destletters;
> > > +    const char *dststr = NULL;
> > > +    uint16_t def_opcode;
> > > +    char letter;
> > > +    int def_regnum;
> >
> > def_regnum has function level scope...
> >
> > > +
> > > +    for (i = 1; i < pkt->num_insns; i++) {
> > > +        uint16_t use_opcode = pkt->insn[i].opcode;
> > > +        if (GET_ATTRIB(use_opcode, A_DOTNEWVALUE) &&
> > > +            GET_ATTRIB(use_opcode, A_CVI) &&
> > > +            GET_ATTRIB(use_opcode, A_STORE)) {
> > > +            int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
> > > +                opcode_reginfo[use_opcode];
> > > +            /*
> > > +             * What's encoded at the N-field is the offset to who's producing
> > > +             * the value.
> > > +             * Shift off the LSB which indicates odd/even register.
> > > +             */
> > > +            int def_off = ((pkt->insn[i].regno[use_regidx]) >> 1);
> > > +            int def_oreg = pkt->insn[i].regno[use_regidx] & 1;
> > > +            int def_idx = -1;
> > > +            for (j = i - 1; (j >= 0) && (def_off >= 0); j--) {
> > > +                if (!GET_ATTRIB(pkt->insn[j].opcode, A_CVI)) {
> > > +                    continue;
> > > +                }
> > > +                def_off--;
> > > +                if (def_off == 0) {
> > > +                    def_idx = j;
> > > +                    break;
> > > +                }
> > > +            }
> > > +            /*
> > > +             * Check for a badly encoded N-field which points to an instruction
> > > +             * out-of-range
> > > +             */
> > > +            g_assert(!((def_off != 0) || (def_idx < 0) ||
> > > +                       (def_idx > (pkt->num_insns - 1))));
> > > +
> > > +            /* def_idx is the index of the producer */
> > > +            def_opcode = pkt->insn[def_idx].opcode;
> > > +            reginfo = opcode_reginfo[def_opcode];
> > > +            destletters = "dexy";
> > > +            for (j = 0; (letter = destletters[j]) != 0; j++) {
> > > +                dststr = strchr(reginfo, letter);
> > > +                if (dststr != NULL) {
> > > +                    break;
> > > +                }
> > > +            }
> > > +            if ((dststr == NULL)  && GET_ATTRIB(def_opcode, A_CVI_GATHER))
> {
> > > +                def_regnum = 0;
> >
> > In this half of the if() we set it to 0...
> >
> > > +                pkt->insn[i].regno[use_regidx] = def_oreg;
> > > +                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
> > > +            } else {
> > > +                if (dststr == NULL) {
> > > +                    /* still not there, we have a bad packet */
> > > +                    g_assert_not_reached();
> > > +                }
> > > +                def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> > > +                /* Now patch up the consumer with the register number */
> > > +                pkt->insn[i].regno[use_regidx] = def_regnum ^ def_oreg;
> > > +                /* special case for (Vx,Vy) */
> > > +                dststr = strchr(reginfo, 'y');
> > > +                if (def_oreg && strchr(reginfo, 'x') && dststr) {
> > > +                    def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
> > > +                    pkt->insn[i].regno[use_regidx] = def_regnum;
> > > +                }
> >
> > ...but the only place we read def_regnum is in this other half of the
> > if(), and if we get here then we've set it to something out of pxt->insn.
> >
> > Were we supposed to do something with def_regnum outside this if(),
> > or could we instead drop the initialization in the first half of the if()
> > and move its declaration inside this else {} block ?
> 
> Hmm -- we'll take a look at this and get back to you.

Yes, I believe that the declaration should move inside and remove the initialization.  I'll prepare a patch to address this.

-Brian
diff mbox series

Patch

diff --git a/target/hexagon/mmvec/decode_ext_mmvec.h b/target/hexagon/mmvec/decode_ext_mmvec.h
new file mode 100644
index 0000000..3664b68
--- /dev/null
+++ b/target/hexagon/mmvec/decode_ext_mmvec.h
@@ -0,0 +1,24 @@ 
+/*
+ *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HEXAGON_DECODE_EXT_MMVEC_H
+#define HEXAGON_DECODE_EXT_MMVEC_H
+
+void mmvec_ext_decode_checks(Packet *pkt, bool disas_only);
+SlotMask mmvec_ext_decode_find_iclass_slots(int opcode);
+
+#endif
diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index d424245..653bfd7 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -22,6 +22,7 @@ 
 #include "decode.h"
 #include "insn.h"
 #include "printinsn.h"
+#include "mmvec/decode_ext_mmvec.h"
 
 #define fZXTN(N, M, VAL) ((VAL) & ((1LL << (N)) - 1))
 
@@ -566,8 +567,12 @@  static void decode_remove_extenders(Packet *packet)
 
 static SlotMask get_valid_slots(const Packet *pkt, unsigned int slot)
 {
-    return find_iclass_slots(pkt->insn[slot].opcode,
-                             pkt->insn[slot].iclass);
+    if (GET_ATTRIB(pkt->insn[slot].opcode, A_EXTENSION)) {
+        return mmvec_ext_decode_find_iclass_slots(pkt->insn[slot].opcode);
+    } else {
+        return find_iclass_slots(pkt->insn[slot].opcode,
+                                 pkt->insn[slot].iclass);
+    }
 }
 
 #define DECODE_NEW_TABLE(TAG, SIZE, WHATNOT)     /* NOTHING */
@@ -728,6 +733,11 @@  decode_insns_tablewalk(Insn *insn, const DectreeTable *table,
         }
         decode_op(insn, opc, encoding);
         return 1;
+    } else if (table->table[i].type == DECTREE_EXTSPACE) {
+        /*
+         * For now, HVX will be the only coproc
+         */
+        return decode_insns_tablewalk(insn, ext_trees[EXT_IDX_mmvec], encoding);
     } else {
         return 0;
     }
@@ -874,6 +884,7 @@  int decode_packet(int max_words, const uint32_t *words, Packet *pkt,
     int words_read = 0;
     bool end_of_packet = false;
     int new_insns = 0;
+    int i;
     uint32_t encoding32;
 
     /* Initialize */
@@ -901,6 +912,11 @@  int decode_packet(int max_words, const uint32_t *words, Packet *pkt,
         return 0;
     }
     pkt->encod_pkt_size_in_bytes = words_read * 4;
+    pkt->pkt_has_hvx = false;
+    for (i = 0; i < num_insns; i++) {
+        pkt->pkt_has_hvx |=
+            GET_ATTRIB(pkt->insn[i].opcode, A_CVI);
+    }
 
     /*
      * Check for :endloop in the parse bits
@@ -931,6 +947,10 @@  int decode_packet(int max_words, const uint32_t *words, Packet *pkt,
     decode_set_slot_number(pkt);
     decode_fill_newvalue_regno(pkt);
 
+    if (pkt->pkt_has_hvx) {
+        mmvec_ext_decode_checks(pkt, disas_only);
+    }
+
     if (!disas_only) {
         decode_shuffle_for_execution(pkt);
         decode_split_cmpjump(pkt);
diff --git a/target/hexagon/mmvec/decode_ext_mmvec.c b/target/hexagon/mmvec/decode_ext_mmvec.c
new file mode 100644
index 0000000..061a65a
--- /dev/null
+++ b/target/hexagon/mmvec/decode_ext_mmvec.c
@@ -0,0 +1,236 @@ 
+/*
+ *  Copyright(c) 2019-2021 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "decode.h"
+#include "opcodes.h"
+#include "insn.h"
+#include "iclass.h"
+#include "mmvec/mmvec.h"
+#include "mmvec/decode_ext_mmvec.h"
+
+static void
+check_new_value(Packet *pkt)
+{
+    /* .new value for a MMVector store */
+    int i, j;
+    const char *reginfo;
+    const char *destletters;
+    const char *dststr = NULL;
+    uint16_t def_opcode;
+    char letter;
+    int def_regnum;
+
+    for (i = 1; i < pkt->num_insns; i++) {
+        uint16_t use_opcode = pkt->insn[i].opcode;
+        if (GET_ATTRIB(use_opcode, A_DOTNEWVALUE) &&
+            GET_ATTRIB(use_opcode, A_CVI) &&
+            GET_ATTRIB(use_opcode, A_STORE)) {
+            int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
+                opcode_reginfo[use_opcode];
+            /*
+             * What's encoded at the N-field is the offset to who's producing
+             * the value.
+             * Shift off the LSB which indicates odd/even register.
+             */
+            int def_off = ((pkt->insn[i].regno[use_regidx]) >> 1);
+            int def_oreg = pkt->insn[i].regno[use_regidx] & 1;
+            int def_idx = -1;
+            for (j = i - 1; (j >= 0) && (def_off >= 0); j--) {
+                if (!GET_ATTRIB(pkt->insn[j].opcode, A_CVI)) {
+                    continue;
+                }
+                def_off--;
+                if (def_off == 0) {
+                    def_idx = j;
+                    break;
+                }
+            }
+            /*
+             * Check for a badly encoded N-field which points to an instruction
+             * out-of-range
+             */
+            g_assert(!((def_off != 0) || (def_idx < 0) ||
+                       (def_idx > (pkt->num_insns - 1))));
+
+            /* def_idx is the index of the producer */
+            def_opcode = pkt->insn[def_idx].opcode;
+            reginfo = opcode_reginfo[def_opcode];
+            destletters = "dexy";
+            for (j = 0; (letter = destletters[j]) != 0; j++) {
+                dststr = strchr(reginfo, letter);
+                if (dststr != NULL) {
+                    break;
+                }
+            }
+            if ((dststr == NULL)  && GET_ATTRIB(def_opcode, A_CVI_GATHER)) {
+                def_regnum = 0;
+                pkt->insn[i].regno[use_regidx] = def_oreg;
+                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
+            } else {
+                if (dststr == NULL) {
+                    /* still not there, we have a bad packet */
+                    g_assert_not_reached();
+                }
+                def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
+                /* Now patch up the consumer with the register number */
+                pkt->insn[i].regno[use_regidx] = def_regnum ^ def_oreg;
+                /* special case for (Vx,Vy) */
+                dststr = strchr(reginfo, 'y');
+                if (def_oreg && strchr(reginfo, 'x') && dststr) {
+                    def_regnum = pkt->insn[def_idx].regno[dststr - reginfo];
+                    pkt->insn[i].regno[use_regidx] = def_regnum;
+                }
+                /*
+                 * We need to remember who produces this value to later
+                 * check if it was dynamically cancelled
+                 */
+                pkt->insn[i].new_value_producer_slot = pkt->insn[def_idx].slot;
+            }
+        }
+    }
+}
+
+/*
+ * We don't want to reorder slot1/slot0 with respect to each other.
+ * So in our shuffling, we don't want to move the .cur / .tmp vmem earlier
+ * Instead, we should move the producing instruction later
+ * But the producing instruction might feed a .new store!
+ * So we may need to move that even later.
+ */
+
+static void
+decode_mmvec_move_cvi_to_end(Packet *pkt, int max)
+{
+    int i;
+    for (i = 0; i < max; i++) {
+        if (GET_ATTRIB(pkt->insn[i].opcode, A_CVI)) {
+            int last_inst = pkt->num_insns - 1;
+            uint16_t last_opcode = pkt->insn[last_inst].opcode;
+
+            /*
+             * If the last instruction is an endloop, move to the one before it
+             * Keep endloop as the last thing always
+             */
+            if ((last_opcode == J2_endloop0) ||
+                (last_opcode == J2_endloop1) ||
+                (last_opcode == J2_endloop01)) {
+                last_inst--;
+            }
+
+            decode_send_insn_to(pkt, i, last_inst);
+            max--;
+            i--;    /* Retry this index now that packet has rotated */
+        }
+    }
+}
+
+static void
+decode_shuffle_for_execution_vops(Packet *pkt)
+{
+    /*
+     * Sort for .new
+     */
+    int i;
+    for (i = 0; i < pkt->num_insns; i++) {
+        uint16_t opcode = pkt->insn[i].opcode;
+        if (GET_ATTRIB(opcode, A_LOAD) &&
+            (GET_ATTRIB(opcode, A_CVI_NEW) ||
+             GET_ATTRIB(opcode, A_CVI_TMP))) {
+            /*
+             * Find prior consuming vector instructions
+             * Move to end of packet
+             */
+            decode_mmvec_move_cvi_to_end(pkt, i);
+            break;
+        }
+    }
+
+    /* Move HVX new value stores to the end of the packet */
+    for (i = 0; i < pkt->num_insns - 1; i++) {
+        uint16_t opcode = pkt->insn[i].opcode;
+        if (GET_ATTRIB(opcode, A_STORE) &&
+            GET_ATTRIB(opcode, A_CVI_NEW) &&
+            !GET_ATTRIB(opcode, A_CVI_SCATTER_RELEASE)) {
+            int last_inst = pkt->num_insns - 1;
+            uint16_t last_opcode = pkt->insn[last_inst].opcode;
+
+            /*
+             * If the last instruction is an endloop, move to the one before it
+             * Keep endloop as the last thing always
+             */
+            if ((last_opcode == J2_endloop0) ||
+                (last_opcode == J2_endloop1) ||
+                (last_opcode == J2_endloop01)) {
+                last_inst--;
+            }
+
+            decode_send_insn_to(pkt, i, last_inst);
+            break;
+        }
+    }
+}
+
+static void
+check_for_vhist(Packet *pkt)
+{
+    pkt->vhist_insn = NULL;
+    for (int i = 0; i < pkt->num_insns; i++) {
+        Insn *insn = &pkt->insn[i];
+        int opcode = insn->opcode;
+        if (GET_ATTRIB(opcode, A_CVI) && GET_ATTRIB(opcode, A_CVI_4SLOT)) {
+                pkt->vhist_insn = insn;
+                return;
+        }
+    }
+}
+
+/*
+ * Public Functions
+ */
+
+SlotMask mmvec_ext_decode_find_iclass_slots(int opcode)
+{
+    if (GET_ATTRIB(opcode, A_CVI_VM)) {
+        /* HVX memory instruction */
+        if (GET_ATTRIB(opcode, A_RESTRICT_SLOT0ONLY)) {
+            return SLOTS_0;
+        } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT1ONLY)) {
+            return SLOTS_1;
+        }
+        return SLOTS_01;
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT2ONLY)) {
+        return SLOTS_2;
+    } else if (GET_ATTRIB(opcode, A_CVI_VX)) {
+        /* HVX multiply instruction */
+        return SLOTS_23;
+    } else if (GET_ATTRIB(opcode, A_CVI_VS_VX)) {
+        /* HVX permute/shift instruction */
+        return SLOTS_23;
+    } else {
+        return SLOTS_0123;
+    }
+}
+
+void mmvec_ext_decode_checks(Packet *pkt, bool disas_only)
+{
+    check_new_value(pkt);
+    if (!disas_only) {
+        decode_shuffle_for_execution_vops(pkt);
+    }
+    check_for_vhist(pkt);
+}
diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index a35eb28..b612431 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -175,6 +175,7 @@  hexagon_ss.add(files(
     'printinsn.c',
     'arch.c',
     'fma_emu.c',
+    'mmvec/decode_ext_mmvec.c',
     'mmvec/system_ext_mmvec.c',
 ))