RE: [PATCH 2/9] Hexagon (target/hexagon) Mark new_read_idx in trans functions

[ltaylorsimpson]

> -Original Message-
> From: Matheus Tavares Bernardino 
> Sent: Tuesday, February 27, 2024 8:20 AM
> To: Taylor Simpson 
> Cc: qemu-devel@nongnu.org; bc...@quicinc.com;
> quic_mathb...@quicinc.com; sidn...@quicinc.com;
> quic_mlie...@quicinc.com; richard.hender...@linaro.org;
> phi...@linaro.org; a...@rev.ng; a...@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
>  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

Re: [PATCH 2/9] Hexagon (target/hexagon) Mark new_read_idx in trans functions

[Matheus Tavares Bernardino]

On Mon, 26 Feb 2024 13:17:15 -0700 Taylor Simpson  
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)