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
@@ -22,39 +22,6 @@
import string
import hex_common
-##
-## Helpers for gen_helper_prototype
-##
-def_helper_types = {
- "N": "s32",
- "O": "s32",
- "P": "s32",
- "M": "s32",
- "C": "s32",
- "R": "s32",
- "V": "ptr",
- "Q": "ptr",
-}
-
-def_helper_types_pair = {
- "R": "s64",
- "C": "s64",
- "S": "s64",
- "G": "s64",
- "V": "ptr",
- "Q": "ptr",
-}
-
-
-def gen_def_helper_opn(f, tag, regtype, regid, i):
- if hex_common.is_pair(regid):
- f.write(f", {def_helper_types_pair[regtype]}")
- elif hex_common.is_single(regid):
- f.write(f", {def_helper_types[regtype]}")
- else:
- hex_common.bad_register(regtype, regid)
-
-
##
## 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 = ""
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"
if numscalarresults > 1:
- ## The helper is bogus when there is more than one result
- f.write(f"DEF_HELPER_1({tag}, void, env)\n")
- else:
- ## Figure out how many arguments the helper will take
- if numscalarresults == 0:
- def_helper_size = len(regs) + len(imms) + numscalarreadwrite + 1
- if hex_common.need_pkt_has_multi_cof(tag):
- def_helper_size += 1
- if hex_common.need_pkt_need_commit(tag):
- def_helper_size += 1
- if hex_common.need_part1(tag):
- def_helper_size += 1
- if hex_common.need_slot(tag):
- def_helper_size += 1
- if hex_common.need_PC(tag):
- def_helper_size += 1
- if hex_common.helper_needs_next_PC(tag):
- def_helper_size += 1
- if hex_common.need_condexec_reg(tag, regs):
- def_helper_size += 1
- f.write(f"DEF_HELPER_{def_helper_size}({tag}")
- ## The return type is void
- f.write(", void")
- else:
- def_helper_size = len(regs) + len(imms) + numscalarreadwrite
- if hex_common.need_pkt_has_multi_cof(tag):
- def_helper_size += 1
- if hex_common.need_pkt_need_commit(tag):
- def_helper_size += 1
- if hex_common.need_part1(tag):
- def_helper_size += 1
- if hex_common.need_slot(tag):
- def_helper_size += 1
- if hex_common.need_PC(tag):
- def_helper_size += 1
- if hex_common.need_condexec_reg(tag, regs):
- def_helper_size += 1
- if hex_common.helper_needs_next_PC(tag):
- def_helper_size += 1
- f.write(f"DEF_HELPER_{def_helper_size}({tag}")
-
- ## Generate the qemu DEF_HELPER type for each result
- ## Iterate over this list twice
- ## - Emit the scalar result
- ## - Emit the vector result
- i = 0
- for regtype, regid in regs:
- if hex_common.is_written(regid):
- if not hex_common.is_hvx_reg(regtype):
- gen_def_helper_opn(f, tag, regtype, regid, i)
- i += 1
+ raise Exception("numscalarresults > 1")
- ## Put the env between the outputs and inputs
- f.write(", env")
- i += 1
+ declared = []
+ declared.append(return_type)
- # Second pass
- for regtype, regid in regs:
- if hex_common.is_written(regid):
- if hex_common.is_hvx_reg(regtype):
- gen_def_helper_opn(f, tag, regtype, regid, i)
- i += 1
-
- ## For conditional instructions, we pass in the destination register
- if "A_CONDEXEC" in hex_common.attribdict[tag]:
- for regtype, regid in regs:
- if hex_common.is_writeonly(regid) and not hex_common.is_hvx_reg(
- regtype
- ):
- gen_def_helper_opn(f, tag, regtype, regid, i)
- i += 1
-
- ## Generate the qemu type for each input operand (regs and immediates)
+ ## Put the env between the outputs and inputs
+ declared.append("env")
+
+ ## For predicated instructions, we pass in the destination register
+ if hex_common.is_predicated(tag):
for regtype, regid in regs:
- if hex_common.is_read(regid):
- if hex_common.is_hvx_reg(regtype) and hex_common.is_readwrite(regid):
- continue
- gen_def_helper_opn(f, tag, regtype, regid, i)
- i += 1
- for immlett, bits, immshift in imms:
- f.write(", s32")
-
- ## Add the arguments for the instruction pkt_has_multi_cof,
- ## pkt_needs_commit, PC, next_PC, slot, and part1 (if needed)
- if hex_common.need_pkt_has_multi_cof(tag):
- f.write(", i32")
- if hex_common.need_pkt_need_commit(tag):
- f.write(', i32')
- if hex_common.need_PC(tag):
- f.write(", i32")
- if hex_common.helper_needs_next_PC(tag):
- f.write(", i32")
- if hex_common.need_slot(tag):
- f.write(", i32")
- if hex_common.need_part1(tag):
- f.write(" , i32")
- f.write(")\n")
+ reg = hex_common.get_register(tag, regtype, regid)
+ if reg.is_writeonly() and not reg.is_hvx_reg():
+ declared.append(reg.helper_proto_type())
+ ## Pass the HVX destination registers
+ for regtype, regid in regs:
+ reg = hex_common.get_register(tag, regtype, regid)
+ if reg.is_written() and reg.is_hvx_reg():
+ declared.append(reg.helper_proto_type())
+ ## Pass the source registers
+ for regtype, regid in regs:
+ reg = hex_common.get_register(tag, regtype, regid)
+ if reg.is_read() and not (reg.is_hvx_reg() and reg.is_readwrite()):
+ declared.append(reg.helper_proto_type())
+ ## Pass the immediates
+ for immlett, bits, immshift in imms:
+ 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")
+
+ arguments = ", ".join(declared)
+ f.write(f"DEF_HELPER_{len(declared) - 1}({tag}, {arguments})\n")
def main():
@@ -195,6 +108,7 @@ def main():
if is_idef_parser_enabled:
hex_common.read_idef_parser_enabled_file(sys.argv[5])
hex_common.calculate_attribs()
+ hex_common.init_registers()
tagregs = hex_common.get_tagregs()
tagimms = hex_common.get_tagimms()
diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index 2f8963db59..4149c2ce91 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -290,13 +290,6 @@ def need_pkt_has_multi_cof(tag):
def need_pkt_need_commit(tag):
return 'A_IMPLICIT_WRITES_USR' in attribdict[tag]
-def need_condexec_reg(tag, regs):
- if "A_CONDEXEC" in attribdict[tag]:
- for regtype, regid in regs:
- if is_writeonly(regid) and not is_hvx_reg(regtype):
- return True
- return False
-
def skip_qemu_helper(tag):
return tag in overrides.keys()
@@ -404,10 +397,12 @@ def is_hvx_reg(self):
return False
class Single(Scalar):
- pass
+ def helper_proto_type(self):
+ return "s32"
class Pair(Scalar):
- pass
+ def helper_proto_type(self):
+ return "s64"
class Hvx:
def is_scalar_reg(self):
@@ -416,6 +411,8 @@ def is_hvx_reg(self):
return True
def hvx_off(self):
return f"{self.reg_tcg()}_off"
+ def helper_proto_type(self):
+ return "ptr"
#
# Every register is either Dest or OldSource or NewSource or ReadWrite
--
2.34.1
> -----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
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> 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
> @@ -22,39 +22,6 @@
> import string
> import hex_common
>
> -##
> -## Helpers for gen_helper_prototype
> -##
> -def_helper_types = {
> - "N": "s32",
> - "O": "s32",
> - "P": "s32",
> - "M": "s32",
> - "C": "s32",
> - "R": "s32",
> - "V": "ptr",
> - "Q": "ptr",
> -}
> -
> -def_helper_types_pair = {
> - "R": "s64",
> - "C": "s64",
> - "S": "s64",
> - "G": "s64",
> - "V": "ptr",
> - "Q": "ptr",
> -}
> -
> -
> -def gen_def_helper_opn(f, tag, regtype, regid, i):
> - if hex_common.is_pair(regid):
> - f.write(f", {def_helper_types_pair[regtype]}")
> - elif hex_common.is_single(regid):
> - f.write(f", {def_helper_types[regtype]}")
> - else:
> - hex_common.bad_register(regtype, regid)
> -
> -
> ##
> ## 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?
>
> if numscalarresults > 1:
> - ## The helper is bogus when there is more than one result
> - f.write(f"DEF_HELPER_1({tag}, void, env)\n")
> - else:
> - ## Figure out how many arguments the helper will take
> - if numscalarresults == 0:
> - def_helper_size = len(regs) + len(imms) + numscalarreadwrite + 1
> - if hex_common.need_pkt_has_multi_cof(tag):
> - def_helper_size += 1
> - if hex_common.need_pkt_need_commit(tag):
> - def_helper_size += 1
> - if hex_common.need_part1(tag):
> - def_helper_size += 1
> - if hex_common.need_slot(tag):
> - def_helper_size += 1
> - if hex_common.need_PC(tag):
> - def_helper_size += 1
> - if hex_common.helper_needs_next_PC(tag):
> - def_helper_size += 1
> - if hex_common.need_condexec_reg(tag, regs):
> - def_helper_size += 1
> - f.write(f"DEF_HELPER_{def_helper_size}({tag}")
> - ## The return type is void
> - f.write(", void")
> - else:
> - def_helper_size = len(regs) + len(imms) + numscalarreadwrite
> - if hex_common.need_pkt_has_multi_cof(tag):
> - def_helper_size += 1
> - if hex_common.need_pkt_need_commit(tag):
> - def_helper_size += 1
> - if hex_common.need_part1(tag):
> - def_helper_size += 1
> - if hex_common.need_slot(tag):
> - def_helper_size += 1
> - if hex_common.need_PC(tag):
> - def_helper_size += 1
> - if hex_common.need_condexec_reg(tag, regs):
> - def_helper_size += 1
> - if hex_common.helper_needs_next_PC(tag):
> - def_helper_size += 1
> - f.write(f"DEF_HELPER_{def_helper_size}({tag}")
> -
> - ## Generate the qemu DEF_HELPER type for each result
> - ## Iterate over this list twice
> - ## - Emit the scalar result
> - ## - Emit the vector result
> - i = 0
> - for regtype, regid in regs:
> - if hex_common.is_written(regid):
> - if not hex_common.is_hvx_reg(regtype):
> - gen_def_helper_opn(f, tag, regtype, regid, i)
> - i += 1
> + raise Exception("numscalarresults > 1")
>
> - ## Put the env between the outputs and inputs
> - f.write(", env")
> - i += 1
> + declared = []
> + declared.append(return_type)
>
> - # Second pass
> - for regtype, regid in regs:
> - if hex_common.is_written(regid):
> - if hex_common.is_hvx_reg(regtype):
> - gen_def_helper_opn(f, tag, regtype, regid, i)
> - i += 1
> -
> - ## For conditional instructions, we pass in the destination register
> - if "A_CONDEXEC" in hex_common.attribdict[tag]:
> - for regtype, regid in regs:
> - if hex_common.is_writeonly(regid) and not hex_common.is_hvx_reg(
> - regtype
> - ):
> - gen_def_helper_opn(f, tag, regtype, regid, i)
> - i += 1
> -
> - ## Generate the qemu type for each input operand (regs and immediates)
> + ## Put the env between the outputs and inputs
> + declared.append("env")
> +
> + ## For predicated instructions, we pass in the destination register
> + if hex_common.is_predicated(tag):
> for regtype, regid in regs:
> - if hex_common.is_read(regid):
> - if hex_common.is_hvx_reg(regtype) and
> hex_common.is_readwrite(regid):
> - continue
> - gen_def_helper_opn(f, tag, regtype, regid, i)
> - i += 1
> - for immlett, bits, immshift in imms:
> - f.write(", s32")
> -
> - ## Add the arguments for the instruction pkt_has_multi_cof,
> - ## pkt_needs_commit, PC, next_PC, slot, and part1 (if needed)
> - if hex_common.need_pkt_has_multi_cof(tag):
> - f.write(", i32")
> - if hex_common.need_pkt_need_commit(tag):
> - f.write(', i32')
> - if hex_common.need_PC(tag):
> - f.write(", i32")
> - if hex_common.helper_needs_next_PC(tag):
> - f.write(", i32")
> - if hex_common.need_slot(tag):
> - f.write(", i32")
> - if hex_common.need_part1(tag):
> - f.write(" , i32")
> - f.write(")\n")
> + reg = hex_common.get_register(tag, regtype, regid)
> + if reg.is_writeonly() and not reg.is_hvx_reg():
> + declared.append(reg.helper_proto_type())
> + ## Pass the HVX destination registers
> + for regtype, regid in regs:
> + reg = hex_common.get_register(tag, regtype, regid)
> + if reg.is_written() and reg.is_hvx_reg():
> + declared.append(reg.helper_proto_type())
> + ## Pass the source registers
> + for regtype, regid in regs:
> + reg = hex_common.get_register(tag, regtype, regid)
> + if reg.is_read() and not (reg.is_hvx_reg() and reg.is_readwrite()):
> + declared.append(reg.helper_proto_type())
> + ## Pass the immediates
> + for immlett, bits, immshift in imms:
> + 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")
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)",
+}
> -----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
> -----Original Message-----
> From: ltaylorsimpson@gmail.com <ltaylorsimpson@gmail.com>
> Sent: Tuesday, December 5, 2023 11:08 AM
> 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
>
>
>
> > -----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
How about this?
In hex_common.py
class HelperArg:
def __init__(self, proto_arg, call_arg, func_arg):
self.proto_arg = proto_arg
self.call_arg = call_arg
self.func_arg = func_arg
def extra_helper_args(tag):
args = []
if need_pkt_has_multi_cof(tag):
args.append(HelperArg(
"i32",
"tcg_constant_tl(ctx->pkt->pkt_has_multi_cof)",
"uint32_t pkt_has_multi_cof"
))
if need_pkt_need_commit(tag):
args.append(HelperArg(
"i32",
"tcg_constant_tl(ctx->need_commit)",
"uint32_t pkt_need_commit"
))
...
return args
In gen_tcg_funcs.py
for extra_arg in hex_common.extra_helper_args(tag):
declared.append(extra_arg.call_arg)
In gen_helper_protos.py
for extra_arg in hex_common.extra_helper_args(tag):
declared.append(extra_arg.proto_arg)
In gen_helper_funcs.py
for extra_arg in hex_common.extra_helper_args(tag):
declared.append(extra_arg.func_arg)
A further refinement would be to have all the registers and immediates create a HelperArg. Then, we could consolidate the logic to create the argument lists in hex_common.py. For example, we pass the destination register as an input to the helper for predicated instructions, and the pointer to the destination for HVX.
Thanks,
Taylor
© 2016 - 2024 Red Hat, Inc.