diff mbox series

[RFC,v3,11/34] Hexagon (target/hexagon) register fields

Message ID 1597765847-16637-12-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 
Declare bitfields within registers such as user status register (USR)

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/reg_fields.h     | 40 +++++++++++++++++++++
 target/hexagon/reg_fields_def.h | 78 +++++++++++++++++++++++++++++++++++++++++
 target/hexagon/reg_fields.c     | 28 +++++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 target/hexagon/reg_fields.h
 create mode 100644 target/hexagon/reg_fields_def.h
 create mode 100644 target/hexagon/reg_fields.c

Comments

Richard Henderson Aug. 26, 2020, 2:29 p.m. UTC | #1 
On 8/18/20 8:50 AM, Taylor Simpson wrote:
> Declare bitfields within registers such as user status register (USR)
> 
> Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
> ---
>  target/hexagon/reg_fields.h     | 40 +++++++++++++++++++++
>  target/hexagon/reg_fields_def.h | 78 +++++++++++++++++++++++++++++++++++++++++
>  target/hexagon/reg_fields.c     | 28 +++++++++++++++
>  3 files changed, 146 insertions(+)
>  create mode 100644 target/hexagon/reg_fields.h
>  create mode 100644 target/hexagon/reg_fields_def.h
>  create mode 100644 target/hexagon/reg_fields.c
> 
> diff --git a/target/hexagon/reg_fields.h b/target/hexagon/reg_fields.h
> new file mode 100644
> index 0000000..cf168f0
> --- /dev/null
> +++ b/target/hexagon/reg_fields.h
> @@ -0,0 +1,40 @@
> +/*
> + *  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_REG_FIELDS_H
> +#define HEXAGON_REG_FIELDS_H
> +
> +#define NUM_GEN_REGS 32

What's this?  It doesn't appear to be field related.

> +extern reg_field_t reg_field_info[];

const.

> +enum reg_fields_enum {

Doesn't follow naming guidelines.  But you don't actually use the name at all,
so better to just drop the name entirely?

> +/* USR fields */
> +DEF_REG_FIELD(USR_OVF,
> +    "ovf", 0, 1,
> +    "Sticky Saturation Overflow - "
> +    "Set when saturation occurs while executing instruction that specifies "
> +    "optional saturation, remains set until explicitly cleared by a USR=Rs "
> +    "instruction.")

Is the description as a string really useful, or even used?
A comment would seem to do just as well, not consume space in the final binary,
and even then seems redundant with the actual architecture manual.


r~
Taylor Simpson Aug. 26, 2020, 11:52 p.m. UTC | #2 
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, August 26, 2020 8:30 AM
> 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 11/34] Hexagon (target/hexagon) register fields
>
> > +#define NUM_GEN_REGS 32
>
> What's this?  It doesn't appear to be field related.

Not needed, will remove it.

> > +extern reg_field_t reg_field_info[];
>
> const.

OK

> > +enum reg_fields_enum {
>
> Doesn't follow naming guidelines.  But you don't actually use the name at all,
> so better to just drop the name entirely?

OK

> > +/* USR fields */
> > +DEF_REG_FIELD(USR_OVF,
> > +    "ovf", 0, 1,
> > +    "Sticky Saturation Overflow - "
> > +    "Set when saturation occurs while executing instruction that specifies "
> > +    "optional saturation, remains set until explicitly cleared by a USR=Rs "
> > +    "instruction.")
>
> Is the description as a string really useful, or even used?
> A comment would seem to do just as well, not consume space in the final
> binary,
> and even then seems redundant with the actual architecture manual.

I thought they help make the code more readable.  You are right that they shouldn't take space in the binary.

I can either change so they don't go into the binary or remove them altogether - guess I'll remove them altogether.
diff mbox series

Patch

diff --git a/target/hexagon/reg_fields.h b/target/hexagon/reg_fields.h
new file mode 100644
index 0000000..cf168f0
--- /dev/null
+++ b/target/hexagon/reg_fields.h
@@ -0,0 +1,40 @@ 
+/*
+ *  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_REG_FIELDS_H
+#define HEXAGON_REG_FIELDS_H
+
+#define NUM_GEN_REGS 32
+
+typedef struct {
+    const char *name;
+    int offset;
+    int width;
+    const char *description;
+} reg_field_t;
+
+extern reg_field_t reg_field_info[];
+
+enum reg_fields_enum {
+#define DEF_REG_FIELD(TAG, NAME, START, WIDTH, DESCRIPTION) \
+    TAG,
+#include "reg_fields_def.h"
+    NUM_REG_FIELDS
+#undef DEF_REG_FIELD
+};
+
+#endif
diff --git a/target/hexagon/reg_fields_def.h b/target/hexagon/reg_fields_def.h
new file mode 100644
index 0000000..ffb18a1
--- /dev/null
+++ b/target/hexagon/reg_fields_def.h
@@ -0,0 +1,78 @@ 
+/*
+ *  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/>.
+ */
+
+/*
+ * For registers that have individual fields, explain them here
+ *   DEF_REG_FIELD(tag,
+ *                 name,
+ *                 bit start offset,
+ *                 width,
+ *                 description
+ */
+
+/* USR fields */
+DEF_REG_FIELD(USR_OVF,
+    "ovf", 0, 1,
+    "Sticky Saturation Overflow - "
+    "Set when saturation occurs while executing instruction that specifies "
+    "optional saturation, remains set until explicitly cleared by a USR=Rs "
+    "instruction.")
+DEF_REG_FIELD(USR_FPINVF,
+    "fpinvf", 1, 1,
+    "Floating-point IEEE Invalid Sticky Flag.")
+DEF_REG_FIELD(USR_FPDBZF,
+    "fpdbzf", 2, 1,
+    "Floating-point IEEE Divide-By-Zero Sticky Flag.")
+DEF_REG_FIELD(USR_FPOVFF,
+    "fpovff", 3, 1,
+    "Floating-point IEEE Overflow Sticky Flag.")
+DEF_REG_FIELD(USR_FPUNFF,
+    "fpunff", 4, 1,
+    "Floating-point IEEE Underflow Sticky Flag.")
+DEF_REG_FIELD(USR_FPINPF,
+    "fpinpf", 5, 1,
+    "Floating-point IEEE Inexact Sticky Flag.")
+
+DEF_REG_FIELD(USR_LPCFG,
+    "lpcfg", 8, 2,
+    "Hardware Loop Configuration: "
+    "Number of loop iterations (0-3) remaining before pipeline predicate "
+    "should be set.")
+
+DEF_REG_FIELD(USR_FPRND,
+    "fprnd", 22, 2,
+    "Rounding Mode for Floating-Point Instructions: "
+    "00: Round to nearest, ties to even (default), "
+    "01: Toward zero, "
+    "10: Downward (toward negative infinity), "
+    "11: Upward (toward positive infinity).")
+
+DEF_REG_FIELD(USR_FPINVE,
+    "fpinve", 25, 1,
+    "Enable trap on IEEE Invalid.")
+DEF_REG_FIELD(USR_FPDBZE,
+    "fpdbze", 26, 1, "Enable trap on IEEE Divide-By-Zero.")
+DEF_REG_FIELD(USR_FPOVFE,
+    "fpovfe", 27, 1,
+    "Enable trap on IEEE Overflow.")
+DEF_REG_FIELD(USR_FPUNFE,
+    "fpunfe", 28, 1,
+    "Enable trap on IEEE Underflow.")
+DEF_REG_FIELD(USR_FPINPE,
+    "fpinpe", 29, 1,
+    "Enable trap on IEEE Inexact.")
+
diff --git a/target/hexagon/reg_fields.c b/target/hexagon/reg_fields.c
new file mode 100644
index 0000000..2a3e4f5a
--- /dev/null
+++ b/target/hexagon/reg_fields.c
@@ -0,0 +1,28 @@ 
+/*
+ *  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 "reg_fields.h"
+
+reg_field_t reg_field_info[] = {
+#define DEF_REG_FIELD(TAG, NAME, START, WIDTH, DESCRIPTION)    \
+      {NAME, START, WIDTH, DESCRIPTION},
+#include "reg_fields_def.h"
+      {NULL, 0, 0}
+#undef DEF_REG_FIELD
+};
+