Check that the value matches opcode_reginfo
Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
target/hexagon/insn.h | 3 ++-
target/hexagon/decode.c | 2 ++
target/hexagon/mmvec/decode_ext_mmvec.c | 2 ++
target/hexagon/gen_trans_funcs.py | 15 ++++++++++-----
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/target/hexagon/insn.h b/target/hexagon/insn.h
index 3e7a22c91e..36502bf056 100644
--- a/target/hexagon/insn.h
+++ b/target/hexagon/insn.h
@@ -1,5 +1,5 @@
/*
- * Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ * Copyright(c) 2019-2024 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
@@ -39,6 +39,7 @@ struct Instruction {
uint32_t slot:3;
uint32_t which_extended:1; /* If has an extender, which immediate */
uint32_t new_value_producer_slot:4;
+ int32_t new_read_idx;
bool part1; /*
* cmp-jumps are split into two insns.
diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index a40210ca1e..4595e30384 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -131,6 +131,8 @@ decode_fill_newvalue_regno(Packet *packet)
use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
opcode_reginfo[use_opcode];
}
+ g_assert(packet->insn[i].new_read_idx != -1 &&
+ packet->insn[i].new_read_idx == use_regidx);
/*
* What's encoded at the N-field is the offset to who's producing
diff --git a/target/hexagon/mmvec/decode_ext_mmvec.c b/target/hexagon/mmvec/decode_ext_mmvec.c
index 202d84c7c0..e9007f5d71 100644
--- a/target/hexagon/mmvec/decode_ext_mmvec.c
+++ b/target/hexagon/mmvec/decode_ext_mmvec.c
@@ -41,6 +41,8 @@ check_new_value(Packet *pkt)
GET_ATTRIB(use_opcode, A_STORE)) {
int use_regidx = strchr(opcode_reginfo[use_opcode], 's') -
opcode_reginfo[use_opcode];
+ g_assert(pkt->insn[i].new_read_idx != -1 &&
+ pkt->insn[i].new_read_idx == use_regidx);
/*
* What's encoded at the N-field is the offset to who's producing
* the value.
diff --git a/target/hexagon/gen_trans_funcs.py b/target/hexagon/gen_trans_funcs.py
index 53e844a44b..79475b2946 100755
--- a/target/hexagon/gen_trans_funcs.py
+++ b/target/hexagon/gen_trans_funcs.py
@@ -84,14 +84,15 @@ def gen_trans_funcs(f):
insn->opcode = {tag};
"""))
- regno = 0
- for reg in regs:
- reg_type = reg[0]
- reg_id = reg[1]
+ new_read_idx = -1
+ for regno, regstruct in enumerate(regs):
+ reg_type, reg_id, _, _ = regstruct
+ reg = hex_common.get_register(tag, reg_type, reg_id)
f.write(code_fmt(f"""\
insn->regno[{regno}] = args->{reg_type}{reg_id};
"""))
- regno += 1
+ if reg.is_read() and reg.is_new():
+ new_read_idx = regno
if len(imms) != 0:
mark_which_imm_extended(f, tag)
@@ -112,6 +113,9 @@ def gen_trans_funcs(f):
insn->immed[{immno}] = args->{imm_type}{imm_letter};
"""))
+ f.write(code_fmt(f"""\
+ insn->new_read_idx = {new_read_idx};
+ """))
f.write(textwrap.dedent(f"""\
return true;
{close_curly}
@@ -120,5 +124,6 @@ def gen_trans_funcs(f):
if __name__ == "__main__":
hex_common.read_semantics_file(sys.argv[1])
+ hex_common.init_registers()
with open(sys.argv[2], "w") as f:
gen_trans_funcs(f)
--
2.34.1On Mon, 26 Feb 2024 13:17:15 -0700 Taylor Simpson <ltaylorsimpson@gmail.com> wrote:
>
> diff --git a/target/hexagon/gen_trans_funcs.py b/target/hexagon/gen_trans_funcs.py
> index 53e844a44b..79475b2946 100755
> --- a/target/hexagon/gen_trans_funcs.py
> +++ b/target/hexagon/gen_trans_funcs.py
> @@ -84,14 +84,15 @@ def gen_trans_funcs(f):
> insn->opcode = {tag};
> """))
>
> - regno = 0
> - for reg in regs:
> - reg_type = reg[0]
> - reg_id = reg[1]
> + new_read_idx = -1
> + for regno, regstruct in enumerate(regs):
> + reg_type, reg_id, _, _ = regstruct
> + reg = hex_common.get_register(tag, reg_type, reg_id)
Nit: since we don't care about the remaining elements of regstruct, we
could simplify (and future-proof) this even further to:
reg_type, reg_id, *_ = regstruct
Or perhaps even eliminate the variable entirely:
for regno, (reg_type, reg_id, *_) in enumerate(regs):
...
> f.write(code_fmt(f"""\
> insn->regno[{regno}] = args->{reg_type}{reg_id};
> """))
> - regno += 1
> + if reg.is_read() and reg.is_new():
> + new_read_idx = regno
>
> if len(imms) != 0:
> mark_which_imm_extended(f, tag)
> -----Original Message-----
> From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Sent: Tuesday, February 27, 2024 8:20 AM
> To: Taylor Simpson <ltaylorsimpson@gmail.com>
> Cc: qemu-devel@nongnu.org; bcain@quicinc.com;
> quic_mathbern@quicinc.com; sidneym@quicinc.com;
> quic_mliebel@quicinc.com; richard.henderson@linaro.org;
> philmd@linaro.org; ale@rev.ng; anjo@rev.ng
> Subject: Re: [PATCH 2/9] Hexagon (target/hexagon) Mark new_read_idx in
> trans functions
>
> On Mon, 26 Feb 2024 13:17:15 -0700 Taylor Simpson
> <ltaylorsimpson@gmail.com> wrote:
> >
> > diff --git a/target/hexagon/gen_trans_funcs.py
> > b/target/hexagon/gen_trans_funcs.py
> > index 53e844a44b..79475b2946 100755
> > --- a/target/hexagon/gen_trans_funcs.py
> > +++ b/target/hexagon/gen_trans_funcs.py
> > @@ -84,14 +84,15 @@ def gen_trans_funcs(f):
> > insn->opcode = {tag};
> > """))
> >
> > - regno = 0
> > - for reg in regs:
> > - reg_type = reg[0]
> > - reg_id = reg[1]
> > + new_read_idx = -1
> > + for regno, regstruct in enumerate(regs):
> > + reg_type, reg_id, _, _ = regstruct
> > + reg = hex_common.get_register(tag, reg_type, reg_id)
>
> Nit: since we don't care about the remaining elements of regstruct, we
could
> simplify (and future-proof) this even further to:
>
> reg_type, reg_id, *_ = regstruct
>
> Or perhaps even eliminate the variable entirely:
>
> for regno, (reg_type, reg_id, *_) in enumerate(regs):
> ...
Let's go with the second option. I'll make the change.
Thanks,
Taylor
© 2016 - 2026 Red Hat, Inc.