Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions

[Richard Henderson <richard.henderson@linaro.org>]

On 8/31/20 11:14 AM, Taylor Simpson wrote:
> 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);

Not quite, see below.

> 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.

Yes.

The fGEN_TCG_A2_add macro does not require nor use that {...} argument.  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?

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...);
}

/*
 * 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~