From: Richard Henderson <richard.henderson@linaro.org>
To: cupertinomiranda@gmail.com, qemu-devel@nongnu.org
Cc: shahab@synopsys.com, linux-snps-arc@lists.infradead.org,
claziss@synopsys.com, cmiranda@synopsys.com
Subject: Re: [PATCH 05/27] arc: TCG instruction generator and hand-definitions
Date: Tue, 6 Apr 2021 20:52:26 -0700 [thread overview]
Message-ID: <7fe5b4af-a4b5-354d-9901-67c414c92eef@linaro.org> (raw)
In-Reply-To: <20210405143138.17016-6-cupertinomiranda@gmail.com>
On 4/5/21 7:31 AM, cupertinomiranda@gmail.com wrote:
> From: Cupertino Miranda <cmiranda@synopsys.com>
>
> Add the most generic parts of TCG constructions. It contains the
> basic infrastructure for fundamental ARC features, such as
> ZOL (zero overhead loops) and delay-slots.
> Also includes hand crafted TCG for more intricate instructions, such
> as vector instructions.
>
> Signed-off-by: Shahab Vahedi <shahab@synopsys.com>
Procedure is that you also sign off on the patches you post.
> +#define REG(x) (cpu_r[x])
Does this really provide any clarity to the code?
> +static inline bool use_goto_tb(const DisasContext *dc, target_ulong dest)
Drop all of the unnecessary inline markers.
The compiler will decide for itself just fine.
> +void gen_goto_tb(const DisasContext *ctx, int n, TCGv dest)
> +{
> + tcg_gen_mov_tl(cpu_pc, dest);
> + tcg_gen_andi_tl(cpu_pcl, dest, ~((target_ulong) 3));
> + if (ctx->base.singlestep_enabled) {
> + gen_helper_debug(cpu_env);
> + } else {
> + tcg_gen_exit_tb(NULL, 0);
> + }
> +}
This doesn't use goto_tb, so perhaps a better name?
It also doesn't use tcg_gen_lookup_and_goto_ptr(), so it could be improved.
> +static void gen_gotoi_tb(const DisasContext *ctx, int n, target_ulong dest)
> +{
> + if (use_goto_tb(ctx, dest)) {
> + tcg_gen_goto_tb(n);
> + tcg_gen_movi_tl(cpu_pc, dest);
> + tcg_gen_movi_tl(cpu_pcl, dest & (~((target_ulong) 3)));
> + tcg_gen_exit_tb(ctx->base.tb, n);
> + } else {
> + tcg_gen_movi_tl(cpu_pc, dest);
> + tcg_gen_movi_tl(cpu_pcl, dest & (~((target_ulong) 3)));
> + if (ctx->base.singlestep_enabled) {
> + gen_helper_debug(cpu_env);
> + }
> + tcg_gen_exit_tb(NULL, 0);
Forgot the else, as above. Also not using lookup.
> + for (i = 0; i < 64; i++) {
> + char name[16];
> +
> + sprintf(name, "r[%d]", i);
> + cpu_r[i] = tcg_global_mem_new(cpu_env,
> + ARC_REG_OFFS(r[i]),
> + strdup(name));
g_strdup_printf, and drop the local variable.
> +}
> +static void arc_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
Mind the whitespace.
> +{
> + /* place holder for now */
> +}
> +
> +static void arc_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
> +{
> + DisasContext *dc = container_of(dcbase, DisasContext, base);
> +
> +
> + tcg_gen_insn_start(dc->base.pc_next);
Similarly.
> +static int arc_gen_INVALID(const DisasContext *ctx)
> +{
> + qemu_log_mask(LOG_UNIMP,
> + "invalid inst @:%08x\n", ctx->cpc);
> + return DISAS_NEXT;
> +}
You need to generate an exception here.
> + buffer[0] = cpu_lduw_code(ctx->env, ctx->cpc);
translator_lduw.
> + length = arc_insn_length(buffer[0], cpu->family);
> +
> + switch (length) {
> + case 2:
> + /* 16-bit instructions. */
> + insn = (uint64_t) buffer[0];
> + break;
> + case 4:
> + /* 32-bit instructions. */
> + buffer[1] = cpu_lduw_code(ctx->env, ctx->cpc + 2);
> + uint32_t buf = (buffer[0] << 16) | buffer[1];
Why use the array buffer[] and not a couple of scalars?
> + if (ctx->insn.limm_p) {
> + ctx->insn.limm = ARRANGE_ENDIAN(true,
> + cpu_ldl_code(ctx->env,
translator_ldl
> +/*
> + * Throw "misaligned" exception if 'addr' is not 32-bit aligned.
> + * This check is done irrelevant of status32.AD bit.
> + */
> +static void check_addr_is_word_aligned(const DisasCtxt *ctx,
> + TCGv addr)
> +{
> + TCGLabel *l1 = gen_new_label();
> + TCGv tmp = tcg_temp_local_new();
> +
> + tcg_gen_andi_tl(tmp, addr, 0x3);
> + tcg_gen_brcondi_tl(TCG_COND_EQ, tmp, 0, l1);
> +
> + tcg_gen_mov_tl(cpu_efa, addr);
> + tcg_gen_movi_tl(cpu_eret, ctx->cpc);
> + tcg_gen_mov_tl(cpu_erbta, cpu_bta);
> +
> + TCGv tcg_index = tcg_const_tl(EXCP_MISALIGNED);
> + TCGv tcg_cause = tcg_const_tl(0x0);
> + TCGv tcg_param = tcg_const_tl(0x0);
> +
> + gen_helper_raise_exception(cpu_env, tcg_index, tcg_cause, tcg_param);
> +
> + gen_set_label(l1);
So.. you shouldn't have to do anything like this.
As far as I can see, there's nothing special about enter/leave. All memory ops
are supposed to be aligned (with double-word ops treated as two word ops for
alignment).
So you should be implementing TCGCPUOps.do_unaligned_access, and set
TARGET_ALIGNED_ONLY in default-configs/.
Quite large this patch. I'll get back to it later...
r~
next prev parent reply other threads:[~2021-04-07 3:53 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-05 14:31 *** ARC port for review *** cupertinomiranda
2021-04-05 14:31 ` [PATCH 01/27] arc: Add initial core cpu files cupertinomiranda
2021-04-07 0:47 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 02/27] arc: Decoder code cupertinomiranda
2021-04-07 1:25 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 03/27] arc: Opcode definitions table cupertinomiranda
2021-04-05 14:31 ` [PATCH 04/27] arc: TCG and decoder glue code and helpers cupertinomiranda
2021-04-07 2:37 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 05/27] arc: TCG instruction generator and hand-definitions cupertinomiranda
2021-04-07 3:52 ` Richard Henderson [this message]
2021-04-07 16:47 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 06/27] arc: semfunc.c tcg code generator cupertinomiranda
2021-04-07 17:14 ` Richard Henderson
2021-04-07 18:33 ` Peter Maydell
2021-04-05 14:31 ` [PATCH 07/27] arc: TCG instruction definitions cupertinomiranda
2021-04-07 19:38 ` Richard Henderson
2021-04-08 0:20 ` Richard Henderson
2021-04-12 14:27 ` Cupertino Miranda
2021-04-05 14:31 ` [PATCH 08/27] arc: Add BCR and AUX registers implementation cupertinomiranda
2021-04-05 14:31 ` [PATCH 09/27] arc: Add IRQ and timer subsystem support cupertinomiranda
2021-04-05 14:31 ` [PATCH 10/27] arc: Add memory management unit (MMU) support cupertinomiranda
2021-04-05 14:31 ` [PATCH 11/27] arc: Add memory protection unit (MPU) support cupertinomiranda
2021-04-05 14:31 ` [PATCH 12/27] arc: Add gdbstub and XML for debugging support cupertinomiranda
2021-04-05 14:31 ` [PATCH 13/27] arc: Add Synopsys ARC emulation boards cupertinomiranda
2021-04-05 14:31 ` [PATCH 14/27] arc: Add support for ARCv2 cupertinomiranda
2021-04-07 20:30 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 15/27] tests/tcg: ARC: Add TCG instruction definition tests cupertinomiranda
2021-04-07 20:38 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 16/27] tests/acceptance: ARC: Add linux boot testing cupertinomiranda
2021-04-07 20:40 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 17/27] arcv3: Core cpu file changes cupertinomiranda
2021-04-05 14:31 ` [PATCH 18/27] arcv3: Decoder code cupertinomiranda
2021-04-07 23:07 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 19/27] arcv3: Opcode definition table cupertinomiranda
2021-04-05 14:31 ` [PATCH 20/27] arcv3: TCG, decoder glue code and helper changes cupertinomiranda
2021-04-07 23:36 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 21/27] arcv3: TCG instruction generator changes cupertinomiranda
2021-04-07 23:43 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 22/27] arcv3: TCG instruction definitions cupertinomiranda
2021-04-07 23:48 ` Richard Henderson
2021-04-05 14:31 ` [PATCH 23/27] arcv3: BCR and AUX register changes cupertinomiranda
2021-04-05 14:31 ` [PATCH 24/27] arcv3: IRQ changes and new MMUv6 WIP cupertinomiranda
2021-04-05 14:31 ` [PATCH 25/27] arcv3: gdbstub changes and new XML files cupertinomiranda
2021-04-05 14:31 ` [PATCH 26/27] arcv3: board changes cupertinomiranda
2021-04-05 14:31 ` [PATCH 27/27] arcv3: Add support for ARCv3 cupertinomiranda
2021-04-06 23:47 ` *** ARC port for review *** Richard Henderson
2021-04-12 14:25 ` Cupertino Miranda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7fe5b4af-a4b5-354d-9901-67c414c92eef@linaro.org \
--to=richard.henderson@linaro.org \
--cc=claziss@synopsys.com \
--cc=cmiranda@synopsys.com \
--cc=cupertinomiranda@gmail.com \
--cc=linux-snps-arc@lists.infradead.org \
--cc=qemu-devel@nongnu.org \
--cc=shahab@synopsys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).