Re: [PATCH 4/4] target/hexagon: idef-parser simplify predicate init
On 06/05/24, ltaylorsimp...@gmail.com wrote:
>
>
> > -Original Message-
> > From: Anton Johansson
> > Sent: Monday, May 6, 2024 1:31 PM
> > To: qemu-devel@nongnu.org
> > Cc: a...@rev.ng; ltaylorsimp...@gmail.com; bc...@quicinc.com
> > Subject: [PATCH 4/4] target/hexagon: idef-parser simplify predicate init
> >
> > Only predicate instruction arguments need to be initialized by
> idef-parser.
> > This commit removes registers from the init_list and simplifies
> > gen_inst_init_args() slightly.
> >
> > Signed-off-by: Anton Johansson
> > ---
> > target/hexagon/idef-parser/idef-parser.y| 2 --
> > target/hexagon/idef-parser/parser-helpers.c | 20 +---
> > 2 files changed, 9 insertions(+), 13 deletions(-)
>
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index bae01c2bb8..33e8f82007 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > @@ -1652,26 +1652,24 @@ void gen_inst(Context *c, GString *iname)
> >
> > void gen_inst_init_args(Context *c, YYLTYPE *locp) {
> > +/* If init_list is NULL arguments have already been initialized */
> > if (!c->inst.init_list) {
> > return;
> > }
> >
> > for (unsigned i = 0; i < c->inst.init_list->len; i++) {
> > HexValue *val = _array_index(c->inst.init_list, HexValue, i);
> > -if (val->type == REGISTER_ARG) {
> > -/* Nothing to do here */
> > -} else if (val->type == PREDICATE) {
> > -char suffix = val->is_dotnew ? 'N' : 'V';
> > -EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
> > - val->pred.id, suffix);
> > -} else {
> > -yyassert(c, locp, false, "Invalid arg type!");
> > -}
> > +yyassert(c, locp, val->type == PREDICATE,
> > + "Only predicates need to be initialized!");
> > +char suffix = val->is_dotnew ? 'N' : 'V';
>
> Declarations should be at the beginning of the function per QEMU coding
> standards.
Agh right!
>
> > +EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
>
> Since you know this is a predicate, the bit_width will always be 32. You
> can hard-code that instead of using %u.
Good point, I'll add a paranoia assertion as well.
//Anton
RE: [PATCH 4/4] target/hexagon: idef-parser simplify predicate init
> -Original Message-
> From: Anton Johansson
> Sent: Monday, May 6, 2024 1:31 PM
> To: qemu-devel@nongnu.org
> Cc: a...@rev.ng; ltaylorsimp...@gmail.com; bc...@quicinc.com
> Subject: [PATCH 4/4] target/hexagon: idef-parser simplify predicate init
>
> Only predicate instruction arguments need to be initialized by
idef-parser.
> This commit removes registers from the init_list and simplifies
> gen_inst_init_args() slightly.
>
> Signed-off-by: Anton Johansson
> ---
> target/hexagon/idef-parser/idef-parser.y| 2 --
> target/hexagon/idef-parser/parser-helpers.c | 20 +---
> 2 files changed, 9 insertions(+), 13 deletions(-)
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> index bae01c2bb8..33e8f82007 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1652,26 +1652,24 @@ void gen_inst(Context *c, GString *iname)
>
> void gen_inst_init_args(Context *c, YYLTYPE *locp) {
> +/* If init_list is NULL arguments have already been initialized */
> if (!c->inst.init_list) {
> return;
> }
>
> for (unsigned i = 0; i < c->inst.init_list->len; i++) {
> HexValue *val = _array_index(c->inst.init_list, HexValue, i);
> -if (val->type == REGISTER_ARG) {
> -/* Nothing to do here */
> -} else if (val->type == PREDICATE) {
> -char suffix = val->is_dotnew ? 'N' : 'V';
> -EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
> - val->pred.id, suffix);
> -} else {
> -yyassert(c, locp, false, "Invalid arg type!");
> -}
> +yyassert(c, locp, val->type == PREDICATE,
> + "Only predicates need to be initialized!");
> +char suffix = val->is_dotnew ? 'N' : 'V';
Declarations should be at the beginning of the function per QEMU coding
standards.
> +EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
Since you know this is a predicate, the bit_width will always be 32. You
can hard-code that instead of using %u.
> + val->pred.id, suffix);
> }
>
> /* Free argument init list once we have initialized everything */
Taylor
[PATCH 4/4] target/hexagon: idef-parser simplify predicate init
Only predicate instruction arguments need to be initialized by
idef-parser. This commit removes registers from the init_list and
simplifies gen_inst_init_args() slightly.
Signed-off-by: Anton Johansson
---
target/hexagon/idef-parser/idef-parser.y| 2 --
target/hexagon/idef-parser/parser-helpers.c | 20 +---
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/target/hexagon/idef-parser/idef-parser.y
b/target/hexagon/idef-parser/idef-parser.y
index cd2612eb8c..9ffb9f9699 100644
--- a/target/hexagon/idef-parser/idef-parser.y
+++ b/target/hexagon/idef-parser/idef-parser.y
@@ -233,8 +233,6 @@ code : '{' statements '}'
argument_decl : REG
{
emit_arg(c, &@1, &$1);
-/* Enqueue register into initialization list */
-g_array_append_val(c->inst.init_list, $1);
}
| PRED
{
diff --git a/target/hexagon/idef-parser/parser-helpers.c
b/target/hexagon/idef-parser/parser-helpers.c
index bae01c2bb8..33e8f82007 100644
--- a/target/hexagon/idef-parser/parser-helpers.c
+++ b/target/hexagon/idef-parser/parser-helpers.c
@@ -1652,26 +1652,24 @@ void gen_inst(Context *c, GString *iname)
/*
- * Initialize declared but uninitialized registers, but only for
- * non-conditional instructions
+ * Initialize declared but uninitialized instruction arguments. Only needed for
+ * predicate arguments, initialization of registers is handled by the Hexagon
+ * frontend.
*/
void gen_inst_init_args(Context *c, YYLTYPE *locp)
{
+/* If init_list is NULL arguments have already been initialized */
if (!c->inst.init_list) {
return;
}
for (unsigned i = 0; i < c->inst.init_list->len; i++) {
HexValue *val = _array_index(c->inst.init_list, HexValue, i);
-if (val->type == REGISTER_ARG) {
-/* Nothing to do here */
-} else if (val->type == PREDICATE) {
-char suffix = val->is_dotnew ? 'N' : 'V';
-EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
- val->pred.id, suffix);
-} else {
-yyassert(c, locp, false, "Invalid arg type!");
-}
+yyassert(c, locp, val->type == PREDICATE,
+ "Only predicates need to be initialized!");
+char suffix = val->is_dotnew ? 'N' : 'V';
+EMIT_HEAD(c, "tcg_gen_movi_i%u(P%c%c, 0);\n", val->bit_width,
+ val->pred.id, suffix);
}
/* Free argument init list once we have initialized everything */
--
2.44.0
