From: "Alex Bennée" <alex.bennee@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
robhenry@microsoft.com, mahmoudabdalghany@outlook.com,
aaron@os.amperecomputing.com, cota@braap.org,
"Paolo Bonzini" <pbonzini@redhat.com>,
kuhn.chenqun@huawei.com, "Alex Bennée" <alex.bennee@linaro.org>
Subject: [PATCH v3 09/23] accel/tcg/plugin-gen: fix the call signature for inline callbacks
Date: Sat, 13 Feb 2021 13:03:11 +0000 [thread overview]
Message-ID: <20210213130325.14781-10-alex.bennee@linaro.org> (raw)
In-Reply-To: <20210213130325.14781-1-alex.bennee@linaro.org>
A recent change to the handling of constants in TCG changed the
pattern of ops emitted for a constant add. We no longer emit a mov and
the constant can be applied directly to the TCG_op_add arguments. This
was causing SEGVs when running the insn plugin with arg=inline. Fix
this by updating copy_add_i64 to do the right thing while also adding
a comment at the top of the append section as an aide memoir if
something like this happens again.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: Emilio G. Cota <cota@braap.org>
Message-Id: <20210210172751.11669-1-alex.bennee@linaro.org>
Message-Id: <20210210221053.18050-10-alex.bennee@linaro.org>
---
accel/tcg/plugin-gen.c | 32 +++++++++++---------------------
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index e5dc9d0ca9..8a1bb801e0 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -320,22 +320,6 @@ static TCGOp *copy_const_ptr(TCGOp **begin_op, TCGOp *op, void *ptr)
return op;
}
-static TCGOp *copy_const_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
-{
- if (TCG_TARGET_REG_BITS == 32) {
- /* 2x mov_i32 */
- op = copy_op(begin_op, op, INDEX_op_mov_i32);
- op->args[1] = tcgv_i32_arg(tcg_constant_i32(v));
- op = copy_op(begin_op, op, INDEX_op_mov_i32);
- op->args[1] = tcgv_i32_arg(tcg_constant_i32(v >> 32));
- } else {
- /* mov_i64 */
- op = copy_op(begin_op, op, INDEX_op_mov_i64);
- op->args[1] = tcgv_i64_arg(tcg_constant_i64(v));
- }
- return op;
-}
-
static TCGOp *copy_extu_tl_i64(TCGOp **begin_op, TCGOp *op)
{
if (TARGET_LONG_BITS == 32) {
@@ -374,14 +358,17 @@ static TCGOp *copy_st_i64(TCGOp **begin_op, TCGOp *op)
return op;
}
-static TCGOp *copy_add_i64(TCGOp **begin_op, TCGOp *op)
+static TCGOp *copy_add_i64(TCGOp **begin_op, TCGOp *op, uint64_t v)
{
if (TCG_TARGET_REG_BITS == 32) {
/* all 32-bit backends must implement add2_i32 */
g_assert(TCG_TARGET_HAS_add2_i32);
op = copy_op(begin_op, op, INDEX_op_add2_i32);
+ op->args[4] = tcgv_i32_arg(tcg_constant_i32(v));
+ op->args[5] = tcgv_i32_arg(tcg_constant_i32(v >> 32));
} else {
op = copy_op(begin_op, op, INDEX_op_add_i64);
+ op->args[2] = tcgv_i64_arg(tcg_constant_i64(v));
}
return op;
}
@@ -431,6 +418,12 @@ static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,
return op;
}
+/*
+ * When we append/replace ops here we are sensitive to changing patterns of
+ * TCGOps generated by the tcg_gen_FOO calls when we generated the
+ * empty callbacks. This will assert very quickly in a debug build as
+ * we assert the ops we are replacing are the correct ones.
+ */
static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
TCGOp *begin_op, TCGOp *op, int *cb_idx)
{
@@ -462,11 +455,8 @@ static TCGOp *append_inline_cb(const struct qemu_plugin_dyn_cb *cb,
/* ld_i64 */
op = copy_ld_i64(&begin_op, op);
- /* const_i64 */
- op = copy_const_i64(&begin_op, op, cb->inline_insn.imm);
-
/* add_i64 */
- op = copy_add_i64(&begin_op, op);
+ op = copy_add_i64(&begin_op, op, cb->inline_insn.imm);
/* st_i64 */
op = copy_st_i64(&begin_op, op);
--
2.20.1
next prev parent reply other threads:[~2021-02-13 13:25 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-13 13:03 [PATCH v3 00/23] plugins/next pre-PR (hwprofile, regression fixes, icount count fix) Alex Bennée
2021-02-13 13:03 ` [PATCH v3 01/23] hw/virtio/pci: include vdev name in registered PCI sections Alex Bennée
2021-02-13 13:03 ` [PATCH v3 02/23] plugins: add API to return a name for a IO device Alex Bennée
2021-02-13 13:03 ` [PATCH v3 03/23] plugins: new hwprofile plugin Alex Bennée
2021-02-13 13:03 ` [PATCH v3 04/23] contrib: Don't use '#' flag of printf format Alex Bennée
2021-02-13 13:03 ` [PATCH v3 05/23] contrib: Fix some code style problems, ERROR: "foo * bar" should be "foo *bar" Alex Bennée
2021-02-13 13:03 ` [PATCH v3 06/23] contrib: Add spaces around operator Alex Bennée
2021-02-13 13:03 ` [PATCH v3 07/23] contrib: space required after that ',' Alex Bennée
2021-02-13 13:03 ` [PATCH v3 08/23] contrib: Open brace '{' following struct go on the same line Alex Bennée
2021-02-13 13:03 ` Alex Bennée [this message]
2021-02-13 13:03 ` [PATCH v3 10/23] exec: Move TranslationBlock typedef to qemu/typedefs.h Alex Bennée
2021-02-13 13:03 ` [PATCH v3 11/23] accel/tcg: Create io_recompile_replay_branch hook Alex Bennée
2021-02-13 13:03 ` [PATCH v3 12/23] target/mips: Create mips_io_recompile_replay_branch Alex Bennée
2021-02-13 13:03 ` [PATCH v3 13/23] target/sh4: Create superh_io_recompile_replay_branch Alex Bennée
2021-02-13 13:03 ` [PATCH v3 14/23] tests/plugin: expand insn test to detect duplicate instructions Alex Bennée
2021-02-13 16:26 ` Richard Henderson
2021-02-13 13:03 ` [PATCH v3 15/23] tests/acceptance: add a new set of tests to exercise plugins Alex Bennée
2021-02-13 13:03 ` [PATCH v3 16/23] accel/tcg: actually cache our partial icount TB Alex Bennée
2021-02-13 13:03 ` [PATCH v3 17/23] accel/tcg: cache single instruction TB on pending replay exception Alex Bennée
2021-02-13 16:29 ` Richard Henderson
2021-02-13 13:03 ` [PATCH v3 18/23] accel/tcg: re-factor non-RAM execution code Alex Bennée
2021-02-13 13:03 ` [PATCH v3 19/23] accel/tcg: remove CF_NOCACHE and special cases Alex Bennée
2021-02-13 13:03 ` [PATCH v3 20/23] accel/tcg: allow plugin instrumentation to be disable via cflags Alex Bennée
2021-02-17 16:30 ` Aaron Lindsay via
2021-02-13 13:03 ` [PATCH v3 21/23] tests/acceptance: add a new tests to detect counting errors Alex Bennée
2021-02-13 13:03 ` [PATCH v3 22/23] tests/plugin: allow memory plugin to do both inline and callbacks Alex Bennée
2021-02-17 17:43 ` Philippe Mathieu-Daudé
2021-02-17 19:59 ` Richard Henderson
2021-02-13 13:03 ` [PATCH v3 23/23] tests/acceptance: add a memory callback check Alex Bennée
2021-02-13 14:17 ` Philippe Mathieu-Daudé
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=20210213130325.14781-10-alex.bennee@linaro.org \
--to=alex.bennee@linaro.org \
--cc=aaron@os.amperecomputing.com \
--cc=cota@braap.org \
--cc=kuhn.chenqun@huawei.com \
--cc=mahmoudabdalghany@outlook.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=robhenry@microsoft.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).