QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-arm <qemu-arm@nongnu.org>, QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH 04/55] target/arm: Add handling for PSR.ECI/ICI
Date: Thu, 10 Jun 2021 11:17:13 +0100	[thread overview]
Message-ID: <CAFEAcA_0P=3fE8arF3j90s1z8A-3TJGnmHjHLThxTALvqt3PXA@mail.gmail.com> (raw)
In-Reply-To: <d54b1117-b25a-ff6f-3166-87fd282b674c@linaro.org>

On Tue, 8 Jun 2021 at 00:33, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/7/21 9:57 AM, Peter Maydell wrote:
> > +void clear_eci_state(DisasContext *s)
> > +{
> > +    /*
> > +     * Clear any ECI/ICI state: used when a load multiple/store
> > +     * multiple insn executes.
> > +     */
> > +    if (s->eci) {
> > +        TCGv_i32 tmp = tcg_temp_new_i32();
> > +        tcg_gen_movi_i32(tmp, 0);
>
> tcg_const_i32 or preferably tcg_constant_i32.

I'll use tcg_const_i32(), yep. (I think I copied this absent-mindedly
from some of the existing code in translate.c that uses tcg_gen_movi_i32().)
Can't use tcg_constant_i32() because store_cpu_field() wants to
tcg_temp_free_i32() its argument.

> > +    if (condexec & 0xf) {
> > +        dc->condexec_mask = (condexec & 0xf) << 1;
> > +        dc->condexec_cond = condexec >> 4;
> > +        dc->eci = 0;
> > +    } else {
> > +        dc->condexec_mask = 0;
> > +        dc->condexec_cond = 0;
> > +        if (arm_feature(env, ARM_FEATURE_M)) {
> > +            dc->eci = condexec >> 4;
> > +        }
>
> This else leaves eci uninitialized.

Strictly speaking it doesn't, because gen_intermediate_code
zero-initializes the DisasContext with a "{ }" struct initializer.
But it's clearer to explicitly initialize here I guess. Fixed.

> >       dc->insn = insn;
> >
> > +    if (dc->eci) {
> > +        /*
> > +         * For M-profile continuable instructions, ECI/ICI handling
> > +         * falls into these cases:
> > +         *  - interrupt-continuable instructions
> > +         *     These are the various load/store multiple insns (both
> > +         *     integer and fp). The ICI bits indicate the register
> > +         *     where the load/store can resume. We make the IMPDEF
> > +         *     choice to always do "instruction restart", ie ignore
> > +         *     the ICI value and always execute the ldm/stm from the
> > +         *     start. So all we need to do is zero PSR.ICI if the
> > +         *     insn executes.
> > +         *  - MVE instructions subject to beat-wise execution
> > +         *     Here the ECI bits indicate which beats have already been
> > +         *     executed, and we must honour this. Each insn of this
> > +         *     type will handle it correctly. We will update PSR.ECI
> > +         *     in the helper function for the insn (some ECI values
> > +         *     mean that the following insn also has been partially
> > +         *     executed).
> > +         *  - Special cases which don't advance ECI
> > +         *     The insns LE, LETP and BKPT leave the ECI/ICI state
> > +         *     bits untouched.
> > +         *  - all other insns (the common case)
> > +         *     Non-zero ECI/ICI means an INVSTATE UsageFault.
> > +         *     We place a rewind-marker here. Insns in the previous
> > +         *     three categories will set a flag in the DisasContext.
> > +         *     If the flag isn't set after we call disas_thumb_insn()
> > +         *     or disas_thumb2_insn() then we know we have a "some other
> > +         *     insn" case. We will rewind to the marker (ie throwing away
> > +         *     all the generated code) and instead emit "take exception".
> > +         */
> > +        dc->eci_handled = false;
>
> This should be done in arm_tr_init_disas_context, I think, unconditionally,
> next to eci.
>
> > +        dc->insn_eci_rewind = tcg_last_op();
>
> I believe that this is identical to dc->insn_start.  Certainly there does not
> seem to be any possibility of any opcodes emitted in between.

There's quite a wide separation between where we set insn_start and here
(we set insn_start in arm_tr_insn_start, then there's whatever the accel/tcg
framework chooses to do between the insn_start callback and the translate_insn
callback, then the arm_pre_translate_insn() code). So I felt that a separate
pointer was easier to reason about.

In fact, looking again at the accel/tcg code, if we rewind to insn_start
that will delete any code emitted by the breakpoint_check hook,
anything emitted by plugin_gen_insn_start(), and anything emitted by
gen_io_start() if this is a CF_LAST_IO insn. I think we want to keep
all of those.

> If you think we should use a different field, then initialize it to null next
> to eci/eci_handled.

Done.

-- PMM


  reply	other threads:[~2021-06-10 10:27 UTC|newest]

Thread overview: 130+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 16:57 [PATCH 00/55] target/arm: First slice of MVE implementation Peter Maydell
2021-06-07 16:57 ` [PATCH 01/55] tcg: Introduce tcg_remove_ops_after Peter Maydell
2021-06-07 16:57 ` [PATCH 02/55] target/arm: Enable FPSCR.QC bit for MVE Peter Maydell
2021-06-07 19:02   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 03/55] target/arm: Handle VPR semantics in existing code Peter Maydell
2021-06-07 21:19   ` Richard Henderson
2021-06-10  9:28     ` Peter Maydell
2021-06-07 16:57 ` [PATCH 04/55] target/arm: Add handling for PSR.ECI/ICI Peter Maydell
2021-06-07 23:33   ` Richard Henderson
2021-06-10 10:17     ` Peter Maydell [this message]
2021-06-10 13:39       ` Richard Henderson
2021-06-07 16:57 ` [PATCH 05/55] target/arm: Let vfp_access_check() handle late NOCP checks Peter Maydell
2021-06-07 23:50   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 06/55] target/arm: Implement MVE LCTP Peter Maydell
2021-06-08  0:05   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 07/55] target/arm: Implement MVE WLSTP insn Peter Maydell
2021-06-08  1:42   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 08/55] target/arm: Implement MVE DLSTP Peter Maydell
2021-06-08  2:56   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 09/55] target/arm: Implement MVE LETP insn Peter Maydell
2021-06-08  3:40   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 10/55] target/arm: Add framework for MVE decode Peter Maydell
2021-06-08  3:59   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 11/55] target/arm: Implement MVE VLDR/VSTR (non-widening forms) Peter Maydell
2021-06-08 21:33   ` Richard Henderson
2021-06-08 21:43     ` Richard Henderson
2021-06-09 10:01     ` Peter Maydell
2021-06-09 17:09       ` Richard Henderson
2021-06-10 14:01     ` Peter Maydell
2021-06-07 16:57 ` [PATCH 12/55] target/arm: Implement widening/narrowing MVE VLDR/VSTR insns Peter Maydell
2021-06-08 21:46   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 13/55] target/arm: Implement MVE VCLZ Peter Maydell
2021-06-08 22:10   ` Richard Henderson
2021-06-10 12:40     ` Peter Maydell
2021-06-10 14:03       ` Richard Henderson
2021-06-07 16:57 ` [PATCH 14/55] target/arm: Implement MVE VCLS Peter Maydell
2021-06-08 22:12   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 15/55] bitops.h: Provide hswap32(), hswap64(), wswap64() swapping operations Peter Maydell
2021-06-08  6:53   ` Philippe Mathieu-Daudé
2021-06-08 22:14   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 16/55] target/arm: Implement MVE VREV16, VREV32, VREV64 Peter Maydell
2021-06-08 22:23   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 17/55] target/arm: Implement MVE VMVN (register) Peter Maydell
2021-06-08 22:27   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 18/55] target/arm: Implement MVE VABS Peter Maydell
2021-06-08 22:34   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 19/55] target/arm: Implement MVE VNEG Peter Maydell
2021-06-08 22:40   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 20/55] target/arm: Implement MVE VDUP Peter Maydell
2021-06-08 23:17   ` Richard Henderson
2021-06-09 10:06     ` Peter Maydell
2021-06-09 17:16       ` Richard Henderson
2021-06-07 16:57 ` [PATCH 21/55] target/arm: Implement MVE VAND, VBIC, VORR, VORN, VEOR Peter Maydell
2021-06-08 23:23   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 22/55] target/arm: Implement MVE VADD, VSUB, VMUL Peter Maydell
2021-06-08 23:25   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 23/55] target/arm: Implement MVE VMULH Peter Maydell
2021-06-08 23:29   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 24/55] target/arm: Implement MVE VRMULH Peter Maydell
2021-06-08 23:33   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 25/55] target/arm: Implement MVE VMAX, VMIN Peter Maydell
2021-06-08 23:35   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 26/55] target/arm: Implement MVE VABD Peter Maydell
2021-06-08 23:39   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 27/55] target/arm: Implement MVE VHADD, VHSUB Peter Maydell
2021-06-08 23:43   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 28/55] target/arm: Implement MVE VMULL Peter Maydell
2021-06-08 23:52   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 29/55] target/arm: Implement MVE VMLALDAV Peter Maydell
2021-06-09  0:46   ` Richard Henderson
2021-06-09  0:46   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 30/55] target/arm: Implement MVE VMLSLDAV Peter Maydell
2021-06-09  0:47   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 31/55] include/qemu/int128.h: Add function to create Int128 from int64_t Peter Maydell
2021-06-08  6:45   ` Philippe Mathieu-Daudé
2021-06-09  0:51   ` Richard Henderson
2021-06-07 16:57 ` [PATCH 32/55] target/arm: Implement MVE VRMLALDAVH, VRMLSLDAVH Peter Maydell
2021-06-09  1:05   ` Richard Henderson
2021-06-14 10:19     ` Peter Maydell
2021-06-07 16:57 ` [PATCH 33/55] target/arm: Implement MVE VADD (scalar) Peter Maydell
2021-06-09 17:58   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 34/55] target/arm: Implement MVE VSUB, VMUL (scalar) Peter Maydell
2021-06-09 18:00   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 35/55] target/arm: Implement MVE VHADD, VHSUB (scalar) Peter Maydell
2021-06-09 18:02   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 36/55] target/arm: Implement MVE VBRSR Peter Maydell
2021-06-09 18:08   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 37/55] target/arm: Implement MVE VPST Peter Maydell
2021-06-09 18:23   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 38/55] target/arm: Implement MVE VQADD and VQSUB Peter Maydell
2021-06-09 18:46   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 39/55] target/arm: Implement MVE VQDMULH and VQRDMULH (scalar) Peter Maydell
2021-06-09 18:58   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 40/55] target/arm: Implement MVE VQDMULL scalar Peter Maydell
2021-06-09 19:11   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 41/55] target/arm: Implement MVE VQDMULH, VQRDMULH (vector) Peter Maydell
2021-06-09 19:13   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 42/55] target/arm: Implement MVE VQADD, VQSUB (vector) Peter Maydell
2021-06-09 19:15   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 43/55] target/arm: Implement MVE VQSHL (vector) Peter Maydell
2021-06-09 19:26   ` Richard Henderson
2021-06-14 11:04     ` Peter Maydell
2021-06-07 16:58 ` [PATCH 44/55] target/arm: Implement MVE VQRSHL Peter Maydell
2021-06-09 19:29   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 45/55] target/arm: Implement MVE VSHL insn Peter Maydell
2021-06-09 19:40   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 46/55] target/arm: Implement MVE VRSHL Peter Maydell
2021-06-09 19:43   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 47/55] target/arm: Implement MVE VQDMLADH and VQRDMLADH Peter Maydell
2021-06-09 20:05   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 48/55] target/arm: Implement MVE VQDMLSDH and VQRDMLSDH Peter Maydell
2021-06-09 20:08   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 49/55] target/arm: Implement MVE VQDMULL (vector) Peter Maydell
2021-06-09 20:20   ` Richard Henderson
2021-06-10 19:08     ` Peter Maydell
2021-06-10 19:34       ` Richard Henderson
2021-06-07 16:58 ` [PATCH 50/55] target/arm: Implement MVE VRHADD Peter Maydell
2021-06-09 20:24   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 51/55] target/arm: Implement MVE VADC, VSBC Peter Maydell
2021-06-09 21:06   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 52/55] target/arm: Implement MVE VCADD Peter Maydell
2021-06-09 21:16   ` Richard Henderson
2021-06-10 19:16     ` Peter Maydell
2021-06-07 16:58 ` [PATCH 53/55] target/arm: Implement MVE VHCADD Peter Maydell
2021-06-10  3:50   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 54/55] target/arm: Implement MVE VADDV Peter Maydell
2021-06-10 14:06   ` Richard Henderson
2021-06-07 16:58 ` [PATCH 55/55] target/arm: Make VMOV scalar <-> gpreg beatwise for MVE Peter Maydell
2021-06-10 14:14   ` Richard Henderson
2021-06-09 14:33 ` [PATCH 00/55] target/arm: First slice of MVE implementation no-reply

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='CAFEAcA_0P=3fE8arF3j90s1z8A-3TJGnmHjHLThxTALvqt3PXA@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).