[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: |
Richard Henderson |
Subject: |
Re: [RFC PATCH v3 30/34] Hexagon (target/hexagon) TCG for instructions with multiple definitions |
Date: |
Thu, 24 Sep 2020 08:03:37 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
On 9/23/20 7:56 PM, Taylor Simpson wrote:
>
>
>>> On 8/31/20 4:10 PM, Taylor Simpson wrote:
>>>>
>>>>
>>>>> -----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
>>>>>
>>> Ho hum. Maybe I'm trying to overthink this too much before tackling the
>>> ultimate goal of full parsing of the SHORTCODE.
>>> 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.
>
> I've made these changes to the generator. I hope you like the results. As
> an example, here is what we generate for the add instruction
>
> DEF_TCG_FUNC(A2_add,
> static void generate_A2_add(
> CPUHexagonState *env,
> DisasContext *ctx,
> insn_t *insn,
> packet_t *pkt)
> {
> TCGv RdV = tcg_temp_local_new();
> const int RdN = insn->regno[0];
> TCGv RsV = hex_gpr[insn->regno[1]];
> TCGv RtV = hex_gpr[insn->regno[2]];
> gen_helper_A2_add(RdV, cpu_env, RsV, RtV);
> gen_log_reg_write(RdN, RdV);
> ctx_log_reg_write(ctx, RdN);
> tcg_temp_free(RdV);
> })
I would be happier if the entire function body were not in a macro. Have you
tried to set a gdb breakpoint in one of these? Once upon a time at least, this
would have resulted in all lines of the function becoming one "source line" in
the debug info.
I also think the full function prototype is unnecessary, and the replication of
"A2_add" undesirable.
I would prefer the function prototype itself to be macro-ized.
E.g.
DEF_TCG_FUNC(A2_add)
{
TCGv RdV = tcg_temp_local_new();
const int RdN = insn->regno[0];
TCGv RsV = hex_gpr[insn->regno[1]];
TCGv RtV = hex_gpr[insn->regno[2]];
gen_helper_A2_add(RdV, cpu_env, RsV, RtV);
gen_log_reg_write(RdN, RdV);
ctx_log_reg_write(ctx, RdN);
tcg_temp_free(RdV);
}
with
#define DEF_TCG_FUNC(TAG) \
static void generate_##TAG(CPUHexagonState *env, \
DisasContext *ctx, \
insn_t *insn, \
packet_t *pkt)
> And here is how the generated file gets used in genptr.c
>
> #define DEF_TCG_FUNC(TAG, GENFN) \
> GENFN
> #include "tcg_funcs_generated.h"
> #undef DEF_TCG_FUNC
>
> /*
> * Not all opcodes have generate_<tag> functions, so initialize
> * the table from the tcg_funcs_generated.h file.
> */
> const semantic_insn_t opcode_genptr[XX_LAST_OPCODE] = {
> #define DEF_TCG_FUNC(TAG, GENFN) \
> [TAG] = generate_##TAG,
> #include "tcg_funcs_generated.h"
> #undef DEF_TCG_FUNC
> };
Obviously, the macro I propose above cannot be directly reused, as you do here.
But I also think we should not try to do so.
You've got a script generating stuff. It can just as easily generate two
different lists. You're trying to do too much with the C preprocessor and too
little with python.
At some point in the v3 thread, I had suggested grepping for some macro in
order to indicate to the python script which tags are implemented manually. My
definition above is easy to look for: exactly one thing on the line, easy
regexp.
> I've also addressed several of the items from Richard's review, so I'll
> resubmit the series once I figure out how to get "make check-tcg" working
> under meson.
Yes, make check-tcg is currently broken, as are a few other check-foo. I've
not yet had the courage to look into it, hoping that someone else will do it
first.
r~