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

[Taylor Simpson]

> From: Anton Johansson  
> Sent: Monday, October 18, 2021 6:42 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 09/12] target/hexagon: import parser for idef-parser
> 
> On 9/7/21 18:08, Taylor Simpson wrote:
> +HexValue gen_round(Context *c,
> +   YYLTYPE *locp,
> +   HexValue *source,
> +   HexValue *position) {
> +yyassert(c, locp, source->bit_width <= 32,
> + "fRNDN not implemented for bit widths > 32!");
> +
> +HexValue src = *source;
> +HexValue pos = *position;
> +
> +HexValue src_width = gen_imm_value(c, locp, src.bit_width, 32);
> +HexValue dst_width = gen_imm_value(c, locp, 64, 32);
> +HexValue a = gen_extend_op(c, locp, _width, _width, , 
> SIGNED);
> Are you sure extending is the right thing to do here?
> I believe so, the fRNDN definition in macros.h also extends here
>   #define fRNDN(A, N) N) == 0) ? (A) : (((fSE32_64(A)) + (1 << ((N) - 
> 1))

OK

Thanks,
Taylor

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

[Anton Johansson via]

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


+HexValue gen_round(Context *c,
+   YYLTYPE *locp,
+   HexValue *source,
+   HexValue *position) {
+yyassert(c, locp, source->bit_width <= 32,
+ "fRNDN not implemented for bit widths > 32!");
+
+HexValue src = *source;
+HexValue pos = *position;
+
+HexValue src_width = gen_imm_value(c, locp, src.bit_width, 32);
+HexValue dst_width = gen_imm_value(c, locp, 64, 32);
+HexValue a = gen_extend_op(c, locp, _width, _width, ,
SIGNED);

Are you sure extending is the right thing to do here?


I believe so, the fRNDN definition in macros.h also extends here

  #define fRNDN(A, N) N) == 0) ? (A) : (((fSE32_64(A)) + (1 << ((N) 
- 1))


--
Anton Johansson,
rev.ng Srls.

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

[Richard Henderson]

On 9/7/21 8:08 PM, Taylor Simpson wrote:

diff --git a/target/hexagon/idef-parser/parser-helpers.h
b/target/hexagon/idef-parser/parser-helpers.h
+#define OUT_IMPL(c, locp, x)\
+_Generic(*x,\
+char:  str_print,   \
+uint8_t:   uint8_print, \
+uint64_t:  uint64_print,\
+int:   int_print,   \
+unsigned:  uint_print,  \
+HexValue:  rvalue_out,  \
+default:   out_assert   \
+)(c, locp, x);


Unless something has changed, qemu requires building with GCC 4.8 which is old 
enough that it doesn't support C11 _Generic.


Something has changed.  The minimum is now 7.5, and we use -std=gnu11 now.
We also use _Generic in several other places.


r~

RE: [PATCH v6 09/12] target/hexagon: import parser 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 09/12] target/hexagon: import parser for idef-parser
> 
> From: Paolo Montesel 
> 
> Signed-off-by: Alessandro Di Federico 
> Signed-off-by: Paolo Montesel 


> diff --git a/target/hexagon/idef-parser/parser-helpers.h
> b/target/hexagon/idef-parser/parser-helpers.h
> +#define OUT_IMPL(c, locp, x)\
> +_Generic(*x,\
> +char:  str_print,   \
> +uint8_t:   uint8_print, \
> +uint64_t:  uint64_print,\
> +int:   int_print,   \
> +unsigned:  uint_print,  \
> +HexValue:  rvalue_out,  \
> +default:   out_assert   \
> +)(c, locp, x);

Unless something has changed, qemu requires building with GCC 4.8 which is old 
enough that it doesn't support C11 _Generic.


> +HexValue gen_bin_cmp(Context *c,
> + YYLTYPE *locp,
> + TCGCond type,
> + HexValue *op1_ptr,
> + HexValue *op2_ptr);
> +
> +/* Code generation functions */

Move this comment above the previous function

> +HexValue gen_bin_op(Context *c,
> +   YYLTYPE *locp,
> +   OpType type,
> +   HexValue *operand1,
> +   HexValue *operand2);
> +
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> new file mode 100644

> +void pre_print(Context *c, YYLTYPE *locp, HexPre *pre, bool is_dotnew)
> +{
> +(void) locp;
> +char suffix = is_dotnew ? 'N' : 'V';
> +EMIT(c, "P%c%c", pre->id, suffix);
> +}

Call this pred_print

> +
> +void reg_compose(Context *c, YYLTYPE *locp, HexReg *reg, char reg_id[5])
> +{
> +switch (reg->type) {
> +case GENERAL_PURPOSE:
> +reg_id[0] = 'R';
> +break;
> +case CONTROL:
> +reg_id[0] = 'C';
> +break;
> +case MODIFIER:
> +reg_id[0] = 'M';
> +break;
> +case DOTNEW:
> +reg_id[0] = 'N';
> +reg_id[1] = reg->id;
> +reg_id[2] = 'N';
> +return;
> +}
> +switch (reg->bit_width) {
> +case 32:
> +reg_id[1] = reg->id;
> +reg_id[2] = 'V';
> +break;
> +case 64:
> +reg_id[1] = reg->id;
> +reg_id[2] = reg->id;
> +reg_id[3] = 'V';
> +break;
> +default:
> +yyassert(c, locp, false, "Unhandled register bit width!\n");
> +}
> +}

It would be better to zero out the result in the above function than having the 
caller do it.

> +
> +static void reg_arg_print(Context *c, YYLTYPE *locp, HexReg *reg)
> +{
> +char reg_id[5] = { 0 };

This is where the caller zero's out the result ...

> +reg_compose(c, locp, reg, reg_id);
> +EMIT(c, "%s", reg_id);
> +}


> +/* Temporary values creation */
> +HexValue gen_tmp(Context *c, YYLTYPE *locp, unsigned bit_width)

You should pass the signedness as an argument rather than assuming it's signed.

> +{
> +HexValue rvalue;
> +memset(, 0, sizeof(HexValue));
> +rvalue.type = TEMP;
> +assert(bit_width == 32 || bit_width == 64);
> +rvalue.bit_width = bit_width;
> +rvalue.signedness = SIGNED;
> +rvalue.is_dotnew = false;
> +rvalue.is_manual = false;
> +rvalue.tmp.index = c->inst.tmp_count;
> +OUT(c, locp, "TCGv_i", _width, " tmp_", >inst.tmp_count,
> +" = tcg_temp_new_i", _width, "();\n");
> +c->inst.tmp_count++;
> +return rvalue;
> +}
> +
> +HexValue gen_tmp_value(Context *c,
> +   YYLTYPE *locp,
> +   const char *value,
> +   unsigned bit_width)

You should pass the signedness as an argument rather than assuming it's signed.

> +{
> +HexValue rvalue;
> +memset(, 0, sizeof(HexValue));
> +rvalue.type = TEMP;
> +rvalue.bit_width = bit_width;
> +rvalue.signedness = SIGNED;
> +rvalue.is_dotnew = false;
> +rvalue.is_manual = false;
> +rvalue.tmp.index = c->inst.tmp_count;
> +OUT(c, locp, "TCGv_i", _width, " tmp_", >inst.tmp_count,
> +" = tcg_const_i", _width, "(&qu

[PATCH v6 09/12] target/hexagon: import parser 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/parser-helpers.h   |  346 +++
 target/hexagon/idef-parser/parser-helpers.c   | 2396 +
 target/hexagon/idef-parser/idef-parser.y  |  975 +++
 target/hexagon/meson.build|   27 +-
 tests/docker/dockerfiles/alpine.docker|1 +
 tests/docker/dockerfiles/centos8.docker   |1 +
 tests/docker/dockerfiles/debian-amd64.docker  |1 +
 tests/docker/dockerfiles/debian10.docker  |2 +
 .../dockerfiles/fedora-i386-cross.docker  |2 +
 .../dockerfiles/fedora-win32-cross.docker |2 +
 .../dockerfiles/fedora-win64-cross.docker |2 +
 tests/docker/dockerfiles/fedora.docker|1 +
 tests/docker/dockerfiles/opensuse-leap.docker |1 +
 tests/docker/dockerfiles/ubuntu.docker|2 +
 tests/docker/dockerfiles/ubuntu1804.docker|1 +
 15 files changed, 3759 insertions(+), 1 deletion(-)
 create mode 100644 target/hexagon/idef-parser/parser-helpers.h
 create mode 100644 target/hexagon/idef-parser/parser-helpers.c
 create mode 100644 target/hexagon/idef-parser/idef-parser.y

diff --git a/target/hexagon/idef-parser/parser-helpers.h 
b/target/hexagon/idef-parser/parser-helpers.h
new file mode 100644
index 00..2d1fc5ab07
--- /dev/null
+++ b/target/hexagon/idef-parser/parser-helpers.h
@@ -0,0 +1,346 @@
+/*
+ * 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
+ * Lesser 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 PARSER_HELPERS_H
+#define PARSER_HELPERS_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "tcg/tcg-cond.h"
+
+#include "idef-parser.tab.h"
+#include "idef-parser.yy.h"
+#include "idef-parser.h"
+
+/* Decomment this to disable yyasserts */
+/* #define NDEBUG */
+
+#define ERR_LINE_CONTEXT 40
+
+#define START_COMMENT "/" "*"
+#define END_COMMENT "*" "/"
+
+void yyerror(YYLTYPE *locp,
+ yyscan_t scanner __attribute__((unused)),
+ Context *c,
+ const char *s);
+
+#ifndef NDEBUG
+#define yyassert(context, locp, condition, msg) \
+if (!(condition)) { \
+yyerror(locp, (context)->scanner, (context), (msg)); \
+}
+#endif
+
+bool is_direct_predicate(HexValue *value);
+
+/* Print functions */
+void str_print(Context *c, YYLTYPE *locp, const char *string);
+
+void uint8_print(Context *c, YYLTYPE *locp, uint8_t *num);
+
+void uint64_print(Context *c, YYLTYPE *locp, uint64_t *num);
+
+void int_print(Context *c, YYLTYPE *locp, int *num);
+
+void uint_print(Context *c, YYLTYPE *locp, unsigned *num);
+
+void tmp_print(Context *c, YYLTYPE *locp, HexTmp *tmp);
+
+void pre_print(Context *c, YYLTYPE *locp, HexPre *pre, bool is_dotnew);
+
+void reg_compose(Context *c, YYLTYPE *locp, HexReg *reg, char reg_id[5]);
+
+void reg_print(Context *c, YYLTYPE *locp, HexReg *reg);
+
+void imm_print(Context *c, YYLTYPE *locp, HexImm *imm);
+
+void var_print(Context *c, YYLTYPE *locp, HexVar *var);
+
+void rvalue_out(Context *c, YYLTYPE *locp, void *pointer);
+
+void out_assert(Context *c, YYLTYPE *locp, void *dummy);
+
+/* Copy output code buffer into stdout */
+void commit(Context *c);
+
+#define OUT_IMPL(c, locp, x)\
+_Generic(*x,\
+char:  str_print,   \
+uint8_t:   uint8_print, \
+uint64_t:  uint64_print,\
+int:   int_print,   \
+unsigned:  uint_print,  \
+HexValue:  rvalue_out,  \
+default:   out_assert   \
+)(c, locp, x);
+
+/* Make a FOREACH macro */
+#define FE_1(c, locp, WHAT, X) WHAT(c, locp, X)
+#define FE_2(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_1(c, locp, WHAT, __VA_ARGS__)
+#define FE_3(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_2(c, locp, WHAT, __VA_ARGS__)
+#define FE_4(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_3(c, locp, WHAT, __VA_ARGS__)
+#define FE_5(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_4(c, locp, WHAT, __VA_ARGS__)
+#define FE_6(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_5(c, locp, WHAT, __VA_ARGS__)
+#define FE_7(c, locp, WHAT, X, ...) \
+WHAT(c, locp, X)FE_6(c,