RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser

[Taylor Simpson]

> From: Anton Johansson  
> Sent: Monday, October 18, 2021 6:37 AM
> To: Taylor Simpson ; Alessandro Di Federico 
> ; qemu-devel@nongnu.org
> Cc: Brian Cain ; bab...@rev.ng; ni...@rev.ng; 
> richard.hender...@linaro.org; Alessandro Di Federico 
> Subject: Re: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
> 
> On 9/7/21 18:08, Taylor Simpson wrote:
> +"fNEWREG"|
> +"fCAST4s"{ yylval->cast.bit_width = 32;
> +   yylval->cast.signedness = SIGNED;
> +   return CAST; }
> This doesn't look right - is fNEWREG the same as fCAST4s?
> We followed the definition of fNEWREG in macros.h where it is given as
>   #define fNEWREG(VAL) ((uint32_t) (VAL))

Well, that's different from fCAST4s.  In particular, one is signed and one is 
unsigned.

> +"fCONSTLL"   { return CONSTLL; }
> +"fCONSTULL"  { return CONSTULL; }
> These can just be converts.
> What is meant by "converts" here? 

Type conversion to int64_t/uint64_t.

> +"fHINTJR(RsV)"   { /* Emit no token */ }
> Put this in the list of IDENTITY above
> Same as for fNEWREG. We followed the definition in macros.h as 
>   #define fHINTJR(TARGET) { /* Not modelled in qemu */ } where it no-ops.

OK, as long as it is no-op.

Thanks,
Taylor

Re: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser

[Anton Johansson via]

On 9/7/21 18:08, Taylor Simpson wrote:


+"fNEWREG"|
+"fCAST4s"{ yylval->cast.bit_width = 32;
+   yylval->cast.signedness = SIGNED;
+   return CAST; }

This doesn't look right - is fNEWREG the same as fCAST4s?


We followed the definition of fNEWREG in macros.h where it is given as

  #define fNEWREG(VAL) ((uint32_t) (VAL))


+"fCONSTLL"   { return CONSTLL; }
+"fCONSTULL"  { return CONSTULL; }

These can just be converts.


What is meant by "converts" here?


+"fHINTJR(RsV)"   { /* Emit no token */ }

Put this in the list of IDENTITY above

Same as for fNEWREG. We followed the definition in macros.h as

  #define fHINTJR(TARGET) { /* Not modelled in qemu */ }

where it no-ops.

--
Anton Johansson,
rev.ng Srls.

RE: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser

[Taylor Simpson]

> -Original Message-
> From: Alessandro Di Federico 
> Sent: Tuesday, July 20, 2021 7:30 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson ; Brian Cain
> ; bab...@rev.ng; ni...@rev.ng;
> richard.hender...@linaro.org; Alessandro Di Federico 
> Subject: [PATCH v6 08/12] target/hexagon: import lexer for idef-parser
> 
> From: Paolo Montesel 
> 
> Signed-off-by: Alessandro Di Federico 
> Signed-off-by: Paolo Montesel 
> ---
>  target/hexagon/idef-parser/idef-parser.h  | 258 +++
>  target/hexagon/idef-parser/idef-parser.lex| 642 ++
>  target/hexagon/meson.build|   4 +
>  tests/docker/dockerfiles/alpine.docker|   1 +
>  tests/docker/dockerfiles/centos8.docker   |   1 +
>  tests/docker/dockerfiles/debian-amd64.docker  |   1 +
>  tests/docker/dockerfiles/debian10.docker  |   1 +
>  .../dockerfiles/fedora-i386-cross.docker  |   1 +
>  .../dockerfiles/fedora-win32-cross.docker |   1 +
>  .../dockerfiles/fedora-win64-cross.docker |   1 +
>  tests/docker/dockerfiles/fedora.docker|   1 +
>  tests/docker/dockerfiles/opensuse-leap.docker |   1 +
>  tests/docker/dockerfiles/ubuntu.docker|   1 +
>  tests/docker/dockerfiles/ubuntu1804.docker|   1 +
>  tests/docker/dockerfiles/ubuntu2004.docker|   2 +

Here you are adding flex to several docker files, then in the next patch you 
add bison.  Create a single patch with the necessary changes to the docker 
files together that is separate from the idef lexer for Hexagon.

>  15 files changed, 917 insertions(+)
>  create mode 100644 target/hexagon/idef-parser/idef-parser.h
>  create mode 100644 target/hexagon/idef-parser/idef-parser.lex
> 
> diff --git a/target/hexagon/idef-parser/idef-parser.h b/target/hexagon/idef-
> parser/idef-parser.h
> new file mode 100644
> +/**
> + * Type of register, assigned to the HexReg.type field
> + */
> +typedef enum {GENERAL_PURPOSE, CONTROL, MODIFIER, DOTNEW}
> RegType;

Should be HexRegType to match the other names in this file (e.g., HexSignedness)

> +/**
> + * Semantic record of the PRE token, identifying a predicate
> + */
> +typedef struct HexPre {
> +char id;/**< Identifier of the predicate 
> */
> +} HexPre;

Call this HexPred - HexPre sounds like something that comes before ...


> diff --git a/target/hexagon/idef-parser/idef-parser.lex
> b/target/hexagon/idef-parser/idef-parser.lex

> +REG_ID_32e|s|d|t|u|v|x|y
> +REG_ID_64ee|ss|dd|tt|uu|vv|xx|yy
> +SYS_ID_32s|d
> +SYS_ID_64ss|dd
> +LOWER_PREd|s|t|u|v|e|x|x

Call this PRED_ID to be consistent with REG_ID_32, etc

> +"R"{REG_ID_32}"V" {
> +   yylval->rvalue.type = REGISTER_ARG;
> +   yylval->rvalue.reg.type = GENERAL_PURPOSE;
> +   yylval->rvalue.reg.id = yytext[1];
> +   yylval->rvalue.reg.bit_width = 32;
> +   yylval->rvalue.bit_width = 32;
> +   yylval->rvalue.is_dotnew = false;
> +   yylval->rvalue.signedness = SIGNED;
> +   return REG; }

> +"in R"{REG_ID_32}"V" {
> +   yylval->rvalue.type = REGISTER_ARG;
> +   yylval->rvalue.reg.type = GENERAL_PURPOSE;
> +   yylval->rvalue.reg.id = yytext[4];
> +   yylval->rvalue.reg.bit_width = 32;
> +   yylval->rvalue.bit_width = 32;
> +   yylval->rvalue.is_dotnew = false;
> +   yylval->rvalue.signedness = SIGNED;
> +   return RREG; }

Are you dropping the "in" here?  This looks the same as the "R"(REG_ID_32}"V" 
case.

Ditto for all the ones below ...

> +"in R"{REG_ID_64}"V" {
> +   yylval->rvalue.type = REGISTER_ARG;
> +   yylval->rvalue.reg.type = GENERAL_PURPOSE;
> +   yylval->rvalue.reg.id = yytext[4];
> +   yylval->rvalue.reg.bit_width = 64;
> +   yylval->rvalue.bit_width = 64;
> +   yylval->rvalue.is_dotnew = false;
> +   yylval->rvalue.signedness = SIGNED;
> +   return RREG; }
> +"in N"{REG_ID_32}"N" {
> +   yylval->rvalue.type = REGISTER_ARG;
> +  

[PATCH v6 08/12] target/hexagon: import lexer for idef-parser

[Alessandro Di Federico via]

From: Paolo Montesel 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/idef-parser/idef-parser.h  | 258 +++
 target/hexagon/idef-parser/idef-parser.lex| 642 ++
 target/hexagon/meson.build|   4 +
 tests/docker/dockerfiles/alpine.docker|   1 +
 tests/docker/dockerfiles/centos8.docker   |   1 +
 tests/docker/dockerfiles/debian-amd64.docker  |   1 +
 tests/docker/dockerfiles/debian10.docker  |   1 +
 .../dockerfiles/fedora-i386-cross.docker  |   1 +
 .../dockerfiles/fedora-win32-cross.docker |   1 +
 .../dockerfiles/fedora-win64-cross.docker |   1 +
 tests/docker/dockerfiles/fedora.docker|   1 +
 tests/docker/dockerfiles/opensuse-leap.docker |   1 +
 tests/docker/dockerfiles/ubuntu.docker|   1 +
 tests/docker/dockerfiles/ubuntu1804.docker|   1 +
 tests/docker/dockerfiles/ubuntu2004.docker|   2 +
 15 files changed, 917 insertions(+)
 create mode 100644 target/hexagon/idef-parser/idef-parser.h
 create mode 100644 target/hexagon/idef-parser/idef-parser.lex

diff --git a/target/hexagon/idef-parser/idef-parser.h 
b/target/hexagon/idef-parser/idef-parser.h
new file mode 100644
index 00..f2c07793c6
--- /dev/null
+++ b/target/hexagon/idef-parser/idef-parser.h
@@ -0,0 +1,258 @@
+/*
+ * Copyright(c) 2019-2021 rev.ng Srls. All Rights Reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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 Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#ifndef IDEF_PARSER_H
+#define IDEF_PARSER_H
+
+#include 
+#include 
+#include 
+#include 
+
+#define TCGV_NAME_SIZE 7
+#define MAX_WRITTEN_REGS 32
+#define OFFSET_STR_LEN 32
+#define ALLOC_LIST_LEN 32
+#define ALLOC_NAME_SIZE 32
+#define INIT_LIST_LEN 32
+#define OUT_BUF_LEN (1024 * 1024)
+#define SIGNATURE_BUF_LEN (128 * 1024)
+#define HEADER_BUF_LEN (128 * 1024)
+
+/* Variadic macros to wrap the buffer printing functions */
+#define EMIT(c, ...) \
+do { \
+g_string_append_printf((c)->out_str, __VA_ARGS__);   \
+} while (0)
+
+#define EMIT_SIG(c, ...)   
\
+do {   
\
+g_string_append_printf((c)->signature_str, __VA_ARGS__);   
\
+} while (0)
+
+#define EMIT_HEAD(c, ...)  
\
+do {   
\
+g_string_append_printf((c)->header_str, __VA_ARGS__);  
\
+} while (0)
+
+/**
+ * Type of register, assigned to the HexReg.type field
+ */
+typedef enum {GENERAL_PURPOSE, CONTROL, MODIFIER, DOTNEW} RegType;
+
+typedef enum { UNKNOWN_SIGNEDNESS, SIGNED, UNSIGNED } HexSignedness;
+
+/**
+ * Semantic record of the REG tokens, identifying registers
+ */
+typedef struct HexReg {
+uint8_t id; /**< Identifier of the register  */
+RegType type;   /**< Type of the register*/
+unsigned bit_width; /**< Bit width of the reg, 32 or 64 bits */
+} HexReg;
+
+/**
+ * Data structure, identifying a TCGv temporary value
+ */
+typedef struct HexTmp {
+int index;  /**< Index of the TCGv temporary value*/
+} HexTmp;
+
+/**
+ * Enum of the possible immediated, an immediate is a value which is known
+ * at tinycode generation time, e.g. an integer value, not a TCGv
+ */
+enum ImmUnionTag {
+I,
+VARIABLE,
+VALUE,
+QEMU_TMP,
+IMM_PC,
+IMM_NPC,
+IMM_CONSTEXT,
+};
+
+/**
+ * Semantic record of the IMM token, identifying an immediate constant
+ */
+typedef struct HexImm {
+union {
+char id;/**< Identifier of the immediate */
+uint64_t value; /**< Immediate value (for VALUE type immediates) */
+uint64_t index; /**< Index of the immediate (for int temp vars)  */
+};
+enum ImmUnionTag type;  /**< Type of the immediate  */
+} HexImm;
+
+/**
+ * Semantic record of the PRE token, identifying a predicate
+ */
+typedef struct HexPre {
+char id;/**< Identifier of the predicate */
+} HexPre;
+
+/**
+ * Semantic record of the SAT token, ident