> -----Original Message-----
> From: Richard Henderson <richard.hender...@linaro.org>
> Sent: Monday, August 31, 2020 8:41 PM
> To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for
> instructions with multiple definitions
>
> On 8/31/20 4:10 PM, Taylor Simpson wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Henderson <richard.hender...@linaro.org>
> >> Sent: Monday, August 31, 2020 1:20 PM
> >> To: Taylor Simpson <tsimp...@quicinc.com>; qemu-devel@nongnu.org
> >> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> >> aleksandar.m.m...@gmail.com; a...@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.
> We're certainly not going to be able to handle e.g. "+" or "if", so it is
> going
> to work only for the most trivial of SHORTCODE.
>
> Though in fact loadrub_pr makes that grade...
The prior version of this series included all the overrides I've written to
date. To reduce the size of this version, I removed most of them and only left
the ones that are essential for correct execution. I plan to submit the others
in subsequent series. Anyway, there are >50 overrides of load/store
instructions that leverage SHORTCODE.
> > 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.
> You need to give me a better example that A2_add, then. Because I see that
> being exactly one line, calling a helper that handles all instructions of the
> same format, passing tcg_gen_add_tl as a callback.
Here's a more complicated example for a predicated post-increment load
Static void generate_L2_ploadrit_pi(CPUHexagonState *env, DisasContext*cts,
insn_t *insn, packet_t *pkt)
{
/* L2_ploadrit_pi */
TCGv EA = tcg_temp_local_new();
int PtN = insn->regno[0];
TCGv PtV = hex_pred[PtN];
int RdN = insn->regno[1];
TCGv RdV = tcg_temp_local_new();
if (!is_preloaded(ctx, RdN)) {
tcg_gen_mov_tl(hex_hew_value[RdN], hex_gpr[RdN]);
}
int RxN = insn->regno[2];
TCGv RxV = tcg_temp_local_new();
if (!is_preloaded(ctx, RxN)) {
tcg_gen_mov_tl(hex_new_value[RdN], hex_gpr[RxN]);
}
int siV = insn->immed[0];
tcg_gen_mov_tl(RxV, hex_gpr[RxN]);
fGEN_TCG_L2_ploadrit_pi({fEA_REG(RxV); if(fLSBOLD(PtV)){ fPM_I(RxV,siV);
fLOAD(1,4,u,EA,RdV);} else {LOAD_CANCEL(EA);}});
gen_log_reg_write(RdN, RdV, insn->slot, 1);
gen_log_reg_write(RxN, RxV, insn->slot, 1);
tcg_temp_free(EA);
tcg_temp_free(RdV);
tcg_temp_free(RxV);
/* L2_ploadrit_pi */
}
> Have a browse through my recent microblaze decodetree conversion. Note
> that
> the basic logical operations are implemented with exactly one source line.
With a helper function, our compares are all one line also
static inline void gen_compare(TCGCond cond, TCGv res, TCGv arg1, TCGv arg2)
{
TCGv one = tcg_const_tl(0xff);
TCGv zero = tcg_const_tl(0);
tcg_gen_movcond_tl(cond, res, arg1, arg2, one, zero);
tcg_temp_free(one);
tcg_temp_free(zero);
}
/* Compare instructions */
#define fGEN_TCG_C2_cmpeq(SHORTCODE) \
gen_compare(TCG_COND_EQ, PdV, RsV, RtV)
#define fGEN_TCG_C4_cmpneq(SHORTCODE) \
gen_compare(TCG_COND_NE, PdV, RsV, RtV)
#define fGEN_TCG_C2_cmpgt(SHORTCODE) \
gen_compare(TCG_COND_GT, PdV, RsV, RtV)
#define fGEN_TCG_C2_cmpgtu(SHORTCODE) \
gen_compare(TCG_COND_GTU, PdV, RsV, RtV)
...
> > 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.
>
> This comes from trying to handle instructions in different ways, but
> represent
> them all the same.
>
> I guess I see the attraction of the magic non-parameters -- you get a
> compilation error if the variable is not present, but are not tied to
> positional parameters.
>
> Ho hum. Maybe I'm trying to overthink this too much before tackling the
> ultimate goal of full parsing of the SHORTCODE.
Alessandro (a...@rev.ng) and Niccolo (ni...@rev.ng) are working on this 😊
> Perhaps the only thing for the short term is to have the generator grep
> genptr.c for "#define fGEN", to choose between the two alternatives: inline
> generation or out-of-line helper generation.
That's certainly doable. It will also be good to implement some of your other
ideas
- Have the generator expand the DECL/READ/WRITE/FREE macros will make the
generated code more readable and we can specialize them for predicated vs
non-predicated instructions which will make translation faster.
- Generate the entire generate_<tag> function instead of just the body will
make the generated code more readable.
