| Message ID | 20240226201722.391879-3-ltaylorsimpson@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hexagon (target/hexagon) Clean up .new decode and scripts | expand |
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): ... > 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
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)
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(-)