[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions w
From: |
Taylor Simpson |
Subject: |
RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions |
Date: |
Mon, 31 Aug 2020 23:10:17 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Monday, August 31, 2020 1:20 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; laurent@vivier.eu; riku.voipio@iki.fi;
> aleksandar.m.mail@gmail.com; ale@rev.ng
> Subject: Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for
> instructions with multiple definitions
>
> The fGEN_TCG_A2_add macro does not require nor use that {...} argument.
The fGEN_TCG_A2_add macro does need that argument, but there are cases that do
need it. Here's an example from gen_tcg.h
#define fGEN_TCG_L2_loadrub_pr(SHORTCODE) SHORTCODE
This is explained in the README, but basically the argument is useful if we can
properly define the macros that it contains to generate TCG.
> What it *does* need are the same arguments as are given to generate_<rtag>. I
> assume you are using those arguments implicitly in your current
> fGEN_TCG_<rtag>
> instances?
Yes
>
> It would be cleanest to only have the generate_* functions.
>
> Either they are written by hand (replacing the current fGEN_TCG_*), or they
> are
> generated. In either case, there's just the one level of indirection from
> opcode_genptr[].
>
> I'd imagine
>
> --- genptr.c
>
> #define DEF_TCG_FUNC(TAG) \
> static void generate_##TAG(CPUHexagonState *env, \
> DisasContext *ctx, insn_t *insn, packet_t *pkt)
>
> /*
> * All IIDs with an explicit implementation,
> * overriding the auto-generated helper functions.
> */
>
> DEF_TCG_FUNC(A2_add)
> {
> /* { RdV=RsV+RtV;} */
> tcg_gen_add_tl(args...);
> }
There's additional generated code before and after the tcg_gen_add_tl. IMO, we
don't want the person who writes an override having to reproduce the generated
code. Assuming we have a definition of fGEN_TCG_A2_add and we have the
generator intelligently expanding the macros, this is what will be generated.
static void generate_A2_add(CPUHexagonState *env, DisasContext *ctx, insn_t
*insn, packet_t *pkt)
{
/* A2_add */
int RdN =insn->regno[0];
TCGv RdV = tcg_temp_local_new();
int RsN = insn->regno[1];
TCGv RsV = hex_gpr[RsN];
int RtN = insn->regno[2];
TCGv RtV = hex_gpr[RtN];
fGEN_TCG_A2_add({ RdV=RsV+RtV;});
gen_log_reg_write(RdN, RdV, insn->slot, 0);
ctx_log_reg_write(ctx, RdN);
tcg_temp_free(RdV);
/* A2_add */
}
If there weren't an override, we'd get this
static void generate_A2_add(CPUHexagonState *env, DisasContext *ctx, insn_t
*insn, packet_t *pkt)
{
/* A2_add */
int RdN =insn->regno[0];
TCGv RdV = tcg_temp_local_new();
int RsN = insn->regno[1];
TCGv RsV = hex_gpr[RsN];
int RtN = insn->regno[2];
TCGv RtV = hex_gpr[RtN];
gen_helper_A2_add(RdV, cpu_env, RsV, RtV); /* Only
difference is this line */
gen_log_reg_write(RdN, RdV, insn->slot, 0);
ctx_log_reg_write(ctx, RdN);
tcg_temp_free(RdV);
/* A2_add */
}
The fGEN_TCG_<tag> macro can also mention the operands of the instruction (RdV,
RsV, RtV in this example).
Unlike the generate_<tag> functions that all have the same signature. The
overrides would have different signatures. This would be more defensive
programming because you know exactly where the variables come from but more
verbose when writing the overrides by hand. Also, note that these need to be
macros in order to take advantage of the SHORTCODE.
In other words, instead of
#define fGEN_TCG_A2_add(SHORTCODE) tcg_gen_add_tl(RdV, RsV, RtV)
We would write
#define fGEN_TCG_A2_add(env, ctx, insn, pkt, RdV, RsV, RtV, SHORTCODE)
tcg_gen_add_tl(RdV, RsV, RtV);
Personally, I prefer the former, but will change to the latter if you feel
strongly.
I'm not married to the fGEN_TCG_<tag> name. DEF_TCG_<tag> would also be fine.
>
> /*
> * Generate calls to the auto-generate helpers,
> * and slot everything into the opcode_genptr table.
> */
> #include "genptr_generated.c.inc"
>
> --- genptr_generated.c.inc
>
> DEF_TCG_FUNC(A4_tlbmatch)
> {
> gen_helper_A4_tlbmatch(args...);
> }
>
> // etc
>
> const SemanticInsn opcode_genptr[] = {
> // All IID's, generated or not.
> };
>
> ---
>
> This leaves genptr.c as the file to grep for '^DEF_TCG_FUNC'.
>
>
> r~
- [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, (continued)
- [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/18
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/28
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/30
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/30
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/30
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/30
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/31
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/31
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Taylor Simpson, 2020/08/31
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/31
- RE: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions,
Taylor Simpson <=
- Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions, Richard Henderson, 2020/08/31
[RFC PATCH v3 33/34] Hexagon (tests/tcg/hexagon) TCG tests, Taylor Simpson, 2020/08/18
Re: [RFC PATCH v3 00/34] Hexagon patch series, no-reply, 2020/08/18
Re: [RFC PATCH v3 00/34] Hexagon patch series, Richard Henderson, 2020/08/28