From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH] target/s390x: move tcg_gen_insn_start to s390x_tr_insn_start
Date: Tue, 12 Oct 2021 07:41:20 +0100 [thread overview]
Message-ID: <87y26ywxqs.fsf@linaro.org> (raw)
In-Reply-To: <20211011185332.166763-1-richard.henderson@linaro.org>
Richard Henderson <richard.henderson@linaro.org> writes:
> We use INDEX_op_insn_start to make the start of instruction boundaries.
> If we don't do it in the .insn_start hook things get confused especially
> now plugins want to use that marking to identify the start of instructions
> and will bomb out if it sees instrumented ops before the first instruction
> boundary.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Well that is considerably simpler than my patch... I'll apply it to the
PR and resubmit ;-)
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> target/s390x/tcg/translate.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index f284870cd2..a2d6fa5cca 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -138,6 +138,7 @@ struct DisasFields {
> struct DisasContext {
> DisasContextBase base;
> const DisasInsn *insn;
> + TCGOp *insn_start;
> DisasFields fields;
> uint64_t ex_value;
> /*
> @@ -6380,8 +6381,8 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
> /* Search for the insn in the table. */
> insn = extract_insn(env, s);
>
> - /* Emit insn_start now that we know the ILEN. */
> - tcg_gen_insn_start(s->base.pc_next, s->cc_op, s->ilen);
> + /* Update insn_start now that we know the ILEN. */
> + tcg_set_insn_start_param(s->insn_start, 2, s->ilen);
>
> /* Not found means unimplemented/illegal opcode. */
> if (insn == NULL) {
> @@ -6552,6 +6553,11 @@ static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs)
>
> static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
> {
> + DisasContext *dc = container_of(dcbase, DisasContext, base);
> +
> + /* Delay the set of ilen until we've read the insn. */
> + tcg_gen_insn_start(dc->base.pc_next, dc->cc_op, 0);
> + dc->insn_start = tcg_last_op();
> }
>
> static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
--
Alex Bennée
prev parent reply other threads:[~2021-10-12 6:45 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-11 18:53 [PATCH] target/s390x: move tcg_gen_insn_start to s390x_tr_insn_start Richard Henderson
2021-10-12 6:41 ` Alex Bennée [this message]
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=87y26ywxqs.fsf@linaro.org \
--to=alex.bennee@linaro.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 NNTP newsgroup(s).