> -----Original Message----- > From: Taylor Simpson <ltaylorsimp...@gmail.com> > Sent: Monday, December 4, 2023 7:53 PM > To: qemu-devel@nongnu.org > Cc: Brian Cain <bc...@quicinc.com>; Matheus Bernardino (QUIC) > <quic_mathb...@quicinc.com>; Sid Manning <sidn...@quicinc.com>; Marco > Liebel (QUIC) <quic_mlie...@quicinc.com>; richard.hender...@linaro.org; > phi...@linaro.org; a...@rev.ng; a...@rev.ng; ltaylorsimp...@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 <ltaylorsimp...@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)", +}