From: <ltaylorsimpson@gmail.com>
To: "'Brian Cain'" <bcain@quicinc.com>, <qemu-devel@nongnu.org>
Cc: "'Matheus Bernardino \(QUIC\)'" <quic_mathbern@quicinc.com>,
"'Sid Manning'" <sidneym@quicinc.com>,
"'Marco Liebel \(QUIC\)'" <quic_mliebel@quicinc.com>,
<richard.henderson@linaro.org>, <philmd@linaro.org>, <ale@rev.ng>,
<anjo@rev.ng>
Subject: RE: [PATCH 3/9] Hexagon (target/hexagon) Make generators object oriented - gen_helper_protos
Date: Tue, 5 Dec 2023 11:08:11 -0600 [thread overview]
Message-ID: <0e5901da279d$a14e8f80$e3ebae80$@gmail.com> (raw)
In-Reply-To: <BN7PR02MB41943B67448AAC160D120ED2B885A@BN7PR02MB4194.namprd02.prod.outlook.com>
> -----Original Message-----
> From: Brian Cain <bcain@quicinc.com>
> Sent: Monday, December 4, 2023 9:46 PM
> To: Taylor Simpson <ltaylorsimpson@gmail.com>; qemu-devel@nongnu.org
> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>; Sid
> Manning <sidneym@quicinc.com>; Marco Liebel (QUIC)
> <quic_mliebel@quicinc.com>; richard.henderson@linaro.org;
> philmd@linaro.org; ale@rev.ng; anjo@rev.ng
> Subject: RE: [PATCH 3/9] Hexagon (target/hexagon) Make generators object
> oriented - gen_helper_protos
>
>
>
> > -----Original Message-----
> > From: Taylor Simpson <ltaylorsimpson@gmail.com>
> > Sent: Monday, December 4, 2023 7:53 PM
> > To: qemu-devel@nongnu.org
> > Cc: Brian Cain <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> > <quic_mathbern@quicinc.com>; Sid Manning <sidneym@quicinc.com>;
> Marco
> > Liebel (QUIC) <quic_mliebel@quicinc.com>;
> > richard.henderson@linaro.org; philmd@linaro.org; ale@rev.ng;
> > anjo@rev.ng; ltaylorsimpson@gmail.com
> > Subject: [PATCH 3/9] Hexagon (target/hexagon) Make generators object
> > oriented - gen_helper_protos
> >
> > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> > ---
> > target/hexagon/gen_helper_protos.py | 184 ++++++++--------------------
> > target/hexagon/hex_common.py | 15 +--
> > 2 files changed, 55 insertions(+), 144 deletions(-)
> >
> > diff --git a/target/hexagon/gen_helper_protos.py
> > b/target/hexagon/gen_helper_protos.py
> > index 131043795a..9277199e1d 100755
> > --- a/target/hexagon/gen_helper_protos.py
> > +++ b/target/hexagon/gen_helper_protos.py
> > ##
> > ## Generate the DEF_HELPER prototype for an instruction
> > ## For A2_add: Rd32=add(Rs32,Rt32)
> > @@ -65,116 +32,62 @@ def gen_helper_prototype(f, tag, tagregs,
> tagimms):
> > regs = tagregs[tag]
> > imms = tagimms[tag]
> >
> > - numresults = 0
> > + ## If there is a scalar result, it is the return type
> > + return_type = ""
>
> Should we use `return_type = None` here?
>
> > numscalarresults = 0
> > - numscalarreadwrite = 0
> > for regtype, regid in regs:
> > - if hex_common.is_written(regid):
> > - numresults += 1
> > - if hex_common.is_scalar_reg(regtype):
> > + reg = hex_common.get_register(tag, regtype, regid)
> > + if reg.is_written() and reg.is_scalar_reg():
> > + return_type = reg.helper_proto_type()
> > numscalarresults += 1
> > - if hex_common.is_readwrite(regid):
> > - if hex_common.is_scalar_reg(regtype):
> > - numscalarreadwrite += 1
> > + if numscalarresults == 0:
> > + return_type = "void"
>
> Should we use `return_type = None` here?
I don't see a point of setting it to None. By the time it gets to the use below, it will definitely have a value. We could initialize it to void instead of "" and skip this check.
> > +
> > + ## Other stuff the helper might need
> > + if hex_common.need_pkt_has_multi_cof(tag):
> > + declared.append("i32")
> > + if hex_common.need_pkt_need_commit(tag):
> > + declared.append("i32")
> > + if hex_common.need_PC(tag):
> > + declared.append("i32")
> > + if hex_common.need_next_PC(tag):
> > + declared.append("i32")
> > + if hex_common.need_slot(tag):
> > + declared.append("i32")
> > + if hex_common.need_part1(tag):
> > + declared.append("i32")
>
> What do you think of this instead? The delta below is on top of this patch.
>
> --- a/target/hexagon/gen_helper_protos.py
> +++ b/target/hexagon/gen_helper_protos.py
> @@ -73,18 +73,9 @@ def gen_helper_prototype(f, tag, tagregs, tagimms):
> declared.append("s32")
>
> ## Other stuff the helper might need
> - if hex_common.need_pkt_has_multi_cof(tag):
> - declared.append("i32")
> - if hex_common.need_pkt_need_commit(tag):
> - declared.append("i32")
> - if hex_common.need_PC(tag):
> - declared.append("i32")
> - if hex_common.need_next_PC(tag):
> - declared.append("i32")
> - if hex_common.need_slot(tag):
> - declared.append("i32")
> - if hex_common.need_part1(tag):
> - declared.append("i32")
> + for stuff in hex_common.other_stuff:
> + if stuff(tag):
> + declared.append('i32')
>
> arguments = ", ".join(declared)
> f.write(f"DEF_HELPER_{len(declared) - 1}({tag}, {arguments})\n") diff --git
> a/target/hexagon/gen_tcg_funcs.py b/target/hexagon/gen_tcg_funcs.py
> index 8c2bc03c10..cb02d91886 100755
> --- a/target/hexagon/gen_tcg_funcs.py
> +++ b/target/hexagon/gen_tcg_funcs.py
> @@ -109,18 +109,9 @@ def gen_tcg_func(f, tag, regs, imms):
>
> declared.append(f"tcg_constant_tl({hex_common.imm_name(immlett)})")
>
> ## Other stuff the helper might need
> - if hex_common.need_pkt_has_multi_cof(tag):
> - declared.append("tcg_constant_tl(ctx->pkt->pkt_has_multi_cof)")
> - if hex_common.need_pkt_need_commit(tag):
> - declared.append("tcg_constant_tl(ctx->need_commit)")
> - if hex_common.need_PC(tag):
> - declared.append("tcg_constant_tl(ctx->pkt->pc)")
> - if hex_common.need_next_PC(tag):
> - declared.append("tcg_constant_tl(ctx->next_PC)")
> - if hex_common.need_slot(tag):
> - declared.append("gen_slotval(ctx)")
> - if hex_common.need_part1(tag):
> - declared.append("tcg_constant_tl(insn->part1)")
> + for stuff, text in hex_common.other_stuff:
> + if stuff(tag):
> + declared.append(text)
>
> arguments = ", ".join(declared)
> f.write(f" gen_helper_{tag}({arguments});\n")
> diff --git a/target/hexagon/hex_common.py
> b/target/hexagon/hex_common.py index 90d61a1b16..954532921d 100755
> --- a/target/hexagon/hex_common.py
> +++ b/target/hexagon/hex_common.py
> @@ -1028,3 +1028,13 @@ def get_register(tag, regtype, regid):
> return registers[f"{regtype}{regid}"]
> else:
> return new_registers[f"{regtype}{regid}"]
> +
> +
> +other_stuff = {
> + need_pkt_has_multi_cof: "tcg_constant_tl(ctx->pkt-
> >pkt_has_multi_cof)",
> + need_pkt_need_commit: "tcg_constant_tl(ctx->need_commit)",
> + need_PC: "tcg_constant_tl(ctx->pkt->pc)",
> + need_next_PC: "tcg_constant_tl(ctx->next_PC)",
> + need_slot: "gen_slotval(ctx)",
> + need_part1: "tcg_constant_tl(insn->part1)", }
Great idea to centralize these in hex_common. There are three parts though. There's the actual helper argument in gen_hepler_funcs.py. I'll think about how best to cover the 3 parts. Let me know if you have ideas as well.
Thanks,
Taylor
next prev parent reply other threads:[~2023-12-05 17:08 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-05 1:52 [PATCH 0/9] Hexagon (target/hexagon) Make generators object oriented Taylor Simpson
2023-12-05 1:52 ` [PATCH 1/9] Hexagon (target/hexagon) Clean up handling of modifier registers Taylor Simpson
2023-12-05 3:04 ` Brian Cain
2023-12-05 1:52 ` [PATCH 2/9] Hexagon (target/hexagon) Make generators object oriented - gen_tcg_funcs Taylor Simpson
2023-12-05 3:01 ` Brian Cain
2023-12-05 1:52 ` [PATCH 3/9] Hexagon (target/hexagon) Make generators object oriented - gen_helper_protos Taylor Simpson
2023-12-05 3:46 ` Brian Cain
2023-12-05 17:08 ` ltaylorsimpson [this message]
2023-12-05 22:59 ` ltaylorsimpson
2023-12-05 1:52 ` [PATCH 4/9] Hexagon (target/hexagon) Make generators object oriented - gen_helper_funcs Taylor Simpson
2023-12-05 1:52 ` [PATCH 5/9] Hexagon (target/hexagon) Make generators object oriented - gen_idef_parser_funcs Taylor Simpson
2023-12-05 1:53 ` [PATCH 6/9] Hexagon (target/hexagon) Make generators object oriented - gen_op_regs Taylor Simpson
2023-12-05 3:02 ` Brian Cain
2023-12-05 1:53 ` [PATCH 7/9] Hexagon (target/hexagon) Make generators object oriented - gen_analyze_funcs Taylor Simpson
2023-12-05 1:53 ` [PATCH 8/9] Hexagon (target/hexagon) Remove unused WRITES_PRED_REG attribute Taylor Simpson
2023-12-05 1:53 ` [PATCH 9/9] Hexagon (target/hexagon) Remove dead functions from hex_common.py Taylor Simpson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='0e5901da279d$a14e8f80$e3ebae80$@gmail.com' \
--to=ltaylorsimpson@gmail.com \
--cc=ale@rev.ng \
--cc=anjo@rev.ng \
--cc=bcain@quicinc.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=quic_mathbern@quicinc.com \
--cc=quic_mliebel@quicinc.com \
--cc=richard.henderson@linaro.org \
--cc=sidneym@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).