> -----Original Message-----
> From: Richard Henderson <richard.hender...@linaro.org>
> Sent: Monday, August 31, 2020 11:29 AM
> 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 10:08 AM, Taylor Simpson wrote:
> > There are some assumptions here I'd like to clarify. When I started this
> > discussion, there seemed to be general resistance to the concept of a
> > generator. Because of this, I made the generator as simple as possible and
> > pushed the complexity and optimization into code that consumes the
> output of
> > the generator. Also, I assumed people wouldn't read the output of the
> > generator, so I didn't focus on making it readable.
>
> Except, at some point, one has to debug this code.
> If the code is unreadable, how do you figure out what's broken?
>
> > Now, it sounds like my assumptions were not correct. You pointed out two
> > things to do in the generator> - Expand the macros inline
> > - Skip the instructions that have overrides
>
> Yes please.
>
> > I addition, there other things I should do if we want the generated files to
> be more readable
> > - Indent the code
>
> Helpful, yes.
>
> I wouldn't worry about nested statements within the *.def files too much,
> except to put each ";" terminated statement on a new line, so that gdb's step
> command goes to the next statement instead of skipping everything all at
> once.
>
> > - Only generate one of the fGEN_TCG_<tag> or gen_helper_<tag>
>
> That would be part of "skip the instructions that have overrides", would it
> not?
Just to be explicit, the code that generates code is generated as
#ifdef fGEN_TCG_A2_add
fGEN_TCG_A2_add({ RdV=RsV+RtV;});
#else
do {
gen_helper_A2_add(RdV, cpu_env, RsV, RtV);
} while (0);
#endif
If we're going to have the generator know if there is an override, this would
be more readable as either
fGEN_TCG_A2_add({ RdV=RsV+RtV;});
or
gen_helper_A2_add(RdV, cpu_env, RsV, RtV);
> > - Generate the declaration of the generate_<tag> function instead of just
> the body
>
> I'm not quite sure what this means.
>
> Aren't the "generate_<tag>" functions private to genptr.c? Why would we
> need a
> separate declaration of them, as opposed to just a definition?
In genptr.c, there is
#define DEF_TCG_FUNC(TAG, GENFN) \
static void generate_##TAG(CPUHexagonState *env, DisasContext *ctx, \
insn_t *insn, packet_t *pkt) \
{ \
GENFN \
}
#include "tcg_funcs_generated.h"
#undef DEF_TCG_FUNC
The generated code only has the body of the function. It would be more
readable to move the static void generate_##TAG ... into the generated code.
