diff mbox series

[RFC,v3,27/34] Hexagon (target/hexagon) instruction classes

Message ID 1597765847-16637-28-git-send-email-tsimpson@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Hexagon patch series | expand

Commit Message

Taylor Simpson Aug. 18, 2020, 3:50 p.m. UTC 
Determine legal VLIW slots for each instruction

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/iclass.h            | 46 ++++++++++++++++++++
 target/hexagon/iclass.c            | 88 ++++++++++++++++++++++++++++++++++++++
 target/hexagon/imported/iclass.def | 52 ++++++++++++++++++++++
 3 files changed, 186 insertions(+)
 create mode 100644 target/hexagon/iclass.h
 create mode 100644 target/hexagon/iclass.c
 create mode 100644 target/hexagon/imported/iclass.def

Comments

Richard Henderson Aug. 29, 2020, 1:37 a.m. UTC | #1 
On 8/18/20 8:50 AM, Taylor Simpson wrote:
> +} iclass_t;
...
> +extern const char *find_iclass_slots(opcode_t opcode, int itype);
...
> +typedef struct {
> +    const char * const slots;
> +} iclass_info_t;

I'll note that you aren't following our CODING_STYLE for types.  Which of these
need to match imported/ and which can you fix.

> +typedef enum {
> +
> +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    ICLASS_FROM_TYPE(TYPE),
> +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
> +#include "imported/iclass.def"
> +#undef DEF_PP_ICLASS32
> +#undef DEF_EE_ICLASS32
> +
> +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    ICLASS_FROM_TYPE(TYPE),
> +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
> +#include "imported/iclass.def"
> +#undef DEF_PP_ICLASS32
> +#undef DEF_EE_ICLASS32

Any particular reason why you're defining one as nothing, and doing this dance
twice?

> +const char *find_iclass_slots(opcode_t opcode, int itype)
> +{
> +    /* There are some exceptions to what the iclass dictates */
> +    if (GET_ATTRIB(opcode, A_ICOP)) {
> +        return "2";

Why are you returning a string and not a simple bitmask?

> +    [ICLASS_FROM_TYPE(TYPE)] = { .slots = #SLOTS },

This could be converted to the bitmask with

enum {
    SLOTS_0  = (1 << 0),
    SLOTS_1  = (1 << 1),
    SLOTS_23 = (1 << 2) | (1 << 3),
    ...
};

static const uint8_t iclass_info[] = {

#define DEF_ICLASS(TYPE, SLOTS) \
    [ICLASS_##TYPE] = SLOTS_##SLOTS
#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS) \
    DEF_ICLASS(TYPE, SLOTS)
#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS) \
    DEF_ICLASS(TYPE, SLOTS)

At which point the ultimate consumer,

>     for (i = 0, slot = 3; i < pkt->num_insns; i++) {
>         valid_slot_str = get_valid_slot_str(pkt, i);
> 
>         while (strchr(valid_slot_str, '0' + slot) == NULL) {
>             slot--;
>         }
>         pkt->insn[i].slot = slot;

becomes a simple integer mask check.


r~
Taylor Simpson Aug. 30, 2020, 8:04 p.m. UTC | #2 
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, August 28, 2020 7:37 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; laurent@vivier.eu; riku.voipio@iki.fi;
> aleksandar.m.mail@gmail.com; ale@rev.ng
> Subject: Re: [RFC PATCH v3 27/34] Hexagon (target/hexagon) instruction
> classes
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > +} iclass_t;
> ...
> > +extern const char *find_iclass_slots(opcode_t opcode, int itype);
> ...
> > +typedef struct {
> > +    const char * const slots;
> > +} iclass_info_t;
>
> I'll note that you aren't following our CODING_STYLE for types.  Which of
> these
> need to match imported/ and which can you fix.

So, this should be CamelCase?  I should be able to fix all of them.

>
> > +typedef enum {
> > +
> > +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)
> ICLASS_FROM_TYPE(TYPE),
> > +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
> > +#include "imported/iclass.def"
> > +#undef DEF_PP_ICLASS32
> > +#undef DEF_EE_ICLASS32
> > +
> > +#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)
> ICLASS_FROM_TYPE(TYPE),
> > +#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
> > +#include "imported/iclass.def"
> > +#undef DEF_PP_ICLASS32
> > +#undef DEF_EE_ICLASS32
>
> Any particular reason why you're defining one as nothing, and doing this
> dance
> twice?

Will investigate.

> > +const char *find_iclass_slots(opcode_t opcode, int itype)
> > +{
> > +    /* There are some exceptions to what the iclass dictates */
> > +    if (GET_ATTRIB(opcode, A_ICOP)) {
> > +        return "2";
>
> Why are you returning a string and not a simple bitmask?

Will fix

> > +    [ICLASS_FROM_TYPE(TYPE)] = { .slots = #SLOTS },
>
> This could be converted to the bitmask with
>
> enum {
>     SLOTS_0  = (1 << 0),
>     SLOTS_1  = (1 << 1),
>     SLOTS_23 = (1 << 2) | (1 << 3),
>     ...
> };
>
> static const uint8_t iclass_info[] = {
>
> #define DEF_ICLASS(TYPE, SLOTS) \
>     [ICLASS_##TYPE] = SLOTS_##SLOTS
> #define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS) \
>     DEF_ICLASS(TYPE, SLOTS)
> #define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS) \
>     DEF_ICLASS(TYPE, SLOTS)
>
> At which point the ultimate consumer,
>
> >     for (i = 0, slot = 3; i < pkt->num_insns; i++) {
> >         valid_slot_str = get_valid_slot_str(pkt, i);
> >
> >         while (strchr(valid_slot_str, '0' + slot) == NULL) {
> >             slot--;
> >         }
> >         pkt->insn[i].slot = slot;
>
> becomes a simple integer mask check.

Will fix
Richard Henderson Aug. 30, 2020, 8:43 p.m. UTC | #3 
On 8/30/20 1:04 PM, Taylor Simpson wrote:
> So, this should be CamelCase?  I should be able to fix all of them.

Yes, they should.  Thanks.


r~
diff mbox series

Patch

diff --git a/target/hexagon/iclass.h b/target/hexagon/iclass.h
new file mode 100644
index 0000000..89288ac
--- /dev/null
+++ b/target/hexagon/iclass.h
@@ -0,0 +1,46 @@ 
+/*
+ *  Copyright(c) 2019-2020 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_ICLASS_H
+#define HEXAGON_ICLASS_H
+
+#include "opcodes.h"
+
+#define ICLASS_FROM_TYPE(TYPE) ICLASS_##TYPE
+
+typedef enum {
+
+#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    ICLASS_FROM_TYPE(TYPE),
+#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
+#include "imported/iclass.def"
+#undef DEF_PP_ICLASS32
+#undef DEF_EE_ICLASS32
+
+#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    ICLASS_FROM_TYPE(TYPE),
+#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
+#include "imported/iclass.def"
+#undef DEF_PP_ICLASS32
+#undef DEF_EE_ICLASS32
+
+    ICLASS_FROM_TYPE(COPROC_VX),
+    ICLASS_FROM_TYPE(COPROC_VMEM),
+    NUM_ICLASSES
+} iclass_t;
+
+extern const char *find_iclass_slots(opcode_t opcode, int itype);
+
+#endif
diff --git a/target/hexagon/iclass.c b/target/hexagon/iclass.c
new file mode 100644
index 0000000..32c2236
--- /dev/null
+++ b/target/hexagon/iclass.c
@@ -0,0 +1,88 @@ 
+/*
+ *  Copyright(c) 2019-2020 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 "iclass.h"
+
+typedef struct {
+    const char * const slots;
+} iclass_info_t;
+
+static const iclass_info_t iclass_info[] = {
+
+#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
+#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS) \
+    [ICLASS_FROM_TYPE(TYPE)] = { .slots = #SLOTS },
+
+#include "imported/iclass.def"
+#undef DEF_PP_ICLASS32
+#undef DEF_EE_ICLASS32
+
+#define DEF_PP_ICLASS32(TYPE, SLOTS, UNITS)    /* nothing */
+#define DEF_EE_ICLASS32(TYPE, SLOTS, UNITS) \
+    [ICLASS_FROM_TYPE(TYPE)] = { .slots = #SLOTS },
+
+#include "imported/iclass.def"
+#undef DEF_PP_ICLASS32
+#undef DEF_EE_ICLASS32
+
+    {0}
+};
+
+const char *find_iclass_slots(opcode_t opcode, int itype)
+{
+    /* There are some exceptions to what the iclass dictates */
+    if (GET_ATTRIB(opcode, A_ICOP)) {
+        return "2";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT0ONLY)) {
+        return "0";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT1ONLY)) {
+        return "1";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT2ONLY)) {
+        return "2";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_SLOT3ONLY)) {
+        return "3";
+    } else if (GET_ATTRIB(opcode, A_COF) &&
+               GET_ATTRIB(opcode, A_INDIRECT) &&
+               !GET_ATTRIB(opcode, A_MEMLIKE) &&
+               !GET_ATTRIB(opcode, A_MEMLIKE_PACKET_RULES)) {
+        return "2";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_NOSLOT1)) {
+        return "0";
+    } else if ((opcode == J2_trap0) ||
+               (opcode == Y2_isync) ||
+               (opcode == J4_hintjumpr)) {
+        return "2";
+    } else if ((itype == ICLASS_V2LDST) && (GET_ATTRIB(opcode, A_STORE))) {
+        return "01";
+    } else if ((itype == ICLASS_V2LDST) && (!GET_ATTRIB(opcode, A_STORE))) {
+        return "01";
+    } else if (GET_ATTRIB(opcode, A_CRSLOT23)) {
+        return "23";
+    } else if (GET_ATTRIB(opcode, A_RESTRICT_PREFERSLOT0)) {
+        return "0";
+    } else if (GET_ATTRIB(opcode, A_SUBINSN)) {
+        return "01";
+    } else if (GET_ATTRIB(opcode, A_CALL)) {
+        return "23";
+    } else if ((opcode == J4_jumpseti) || (opcode == J4_jumpsetr)) {
+        return "23";
+    } else {
+        return iclass_info[itype].slots;
+    }
+}
+
diff --git a/target/hexagon/imported/iclass.def b/target/hexagon/imported/iclass.def
new file mode 100644
index 0000000..4ef725f
--- /dev/null
+++ b/target/hexagon/imported/iclass.def
@@ -0,0 +1,52 @@ 
+/*
+ *  Copyright(c) 2019-2020 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/>.
+ */
+
+/* DEF_*(TYPE,SLOTS,UNITS) */
+DEF_PP_ICLASS32(EXTENDER,0123,LDST|SUNIT|MUNIT) /* 0 */
+DEF_PP_ICLASS32(CJ,0123,CTRLFLOW) /* 1 */
+DEF_PP_ICLASS32(NCJ,01,LDST|CTRLFLOW) /* 2 */
+DEF_PP_ICLASS32(V4LDST,01,LDST) /* 3 */
+DEF_PP_ICLASS32(V2LDST,01,LDST) /* 4 */
+DEF_PP_ICLASS32(J,0123,CTRLFLOW)  /* 5 */
+DEF_PP_ICLASS32(CR,3,SUNIT)     /* 6 */
+DEF_PP_ICLASS32(ALU32_2op,0123,LDST|SUNIT|MUNIT) /* 7 */
+DEF_PP_ICLASS32(S_2op,23,SUNIT|MUNIT)               /* 8 */
+DEF_PP_ICLASS32(LD,01,LDST)                    /* 9 */
+DEF_PP_ICLASS32(ST,01,LDST)                        /* 10 */
+DEF_PP_ICLASS32(ALU32_ADDI,0123,LDST|SUNIT|MUNIT) /* 11 */
+DEF_PP_ICLASS32(S_3op,23,SUNIT|MUNIT)               /* 12 */
+DEF_PP_ICLASS32(ALU64,23,SUNIT|MUNIT)             /* 13 */
+DEF_PP_ICLASS32(M,23,SUNIT|MUNIT)                 /* 14 */
+DEF_PP_ICLASS32(ALU32_3op,0123,LDST|SUNIT|MUNIT) /* 15 */
+
+DEF_EE_ICLASS32(EE0,01,INVALID) /* 0 */
+DEF_EE_ICLASS32(EE1,01,INVALID) /* 1 */
+DEF_EE_ICLASS32(EE2,01,INVALID) /* 2 */
+DEF_EE_ICLASS32(EE3,01,INVALID) /* 3 */
+DEF_EE_ICLASS32(EE4,01,INVALID) /* 4 */
+DEF_EE_ICLASS32(EE5,01,INVALID) /* 5 */
+DEF_EE_ICLASS32(EE6,01,INVALID) /* 6 */
+DEF_EE_ICLASS32(EE7,01,INVALID) /* 7 */
+DEF_EE_ICLASS32(EE8,01,INVALID) /* 8 */
+DEF_EE_ICLASS32(EE9,01,INVALID) /* 9 */
+DEF_EE_ICLASS32(EEA,01,INVALID) /* 10 */
+DEF_EE_ICLASS32(EEB,01,INVALID) /* 11 */
+DEF_EE_ICLASS32(EEC,01,INVALID) /* 12 */
+DEF_EE_ICLASS32(EED,01,INVALID) /* 13 */
+DEF_EE_ICLASS32(EEE,01,INVALID) /* 14 */
+DEF_EE_ICLASS32(EEF,01,INVALID) /* 15 */
+