From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org, aurelien@aurel32.net
Subject: Re: [Qemu-devel] [PATCH 03/16] tcg: Propagate args to op->args in tcg.c
Date: Mon, 26 Jun 2017 16:02:23 +0100 [thread overview]
Message-ID: <8760fiyeps.fsf@linaro.org> (raw)
In-Reply-To: <20170621024831.26019-4-rth@twiddle.net>
Richard Henderson <rth@twiddle.net> writes:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/tcg.c | 121 ++++++++++++++++++++++++++++++--------------------------------
> 1 file changed, 58 insertions(+), 63 deletions(-)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 298aa0c..be5b69c 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -1054,14 +1054,12 @@ void tcg_dump_ops(TCGContext *s)
> for (oi = s->gen_op_buf[0].next; oi != 0; oi = op->next) {
> int i, k, nb_oargs, nb_iargs, nb_cargs;
> const TCGOpDef *def;
> - const TCGArg *args;
> TCGOpcode c;
> int col = 0;
>
> op = &s->gen_op_buf[oi];
> c = op->opc;
> def = &tcg_op_defs[c];
> - args = op->args;
>
> if (c == INDEX_op_insn_start) {
> col += qemu_log("%s ----", oi != s->gen_op_buf[0].next ? "\n" : "");
> @@ -1069,9 +1067,9 @@ void tcg_dump_ops(TCGContext *s)
> for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
> target_ulong a;
> #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
> - a = ((target_ulong)args[i * 2 + 1] << 32) | args[i * 2];
> + a = deposit64(op->args[i * 2], 32, 32, op->args[i * 2
> + 1]);
It doesn't now but should be assert against us overflowing the args
buffer here when dealing with encoded data? Or should it have faulted
when planting the ops?
> #else
> - a = args[i];
> + a = op->args[i];
> #endif
> col += qemu_log(" " TARGET_FMT_lx, a);
> }
> @@ -1083,14 +1081,14 @@ void tcg_dump_ops(TCGContext *s)
>
> /* function name, flags, out args */
> col += qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name,
> - tcg_find_helper(s, args[nb_oargs + nb_iargs]),
> - args[nb_oargs + nb_iargs + 1], nb_oargs);
> + tcg_find_helper(s, op->args[nb_oargs + nb_iargs]),
> + op->args[nb_oargs + nb_iargs + 1], nb_oargs);
> for (i = 0; i < nb_oargs; i++) {
> col += qemu_log(",%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
> - args[i]));
> + op->args[i]));
> }
> for (i = 0; i < nb_iargs; i++) {
> - TCGArg arg = args[nb_oargs + i];
> + TCGArg arg = op->args[nb_oargs + i];
> const char *t = "<dummy>";
> if (arg != TCG_CALL_DUMMY_ARG) {
> t = tcg_get_arg_str_idx(s, buf, sizeof(buf), arg);
> @@ -1110,14 +1108,14 @@ void tcg_dump_ops(TCGContext *s)
> col += qemu_log(",");
> }
> col += qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
> - args[k++]));
> + op->args[k++]));
> }
> for (i = 0; i < nb_iargs; i++) {
> if (k != 0) {
> col += qemu_log(",");
> }
> col += qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
> - args[k++]));
> + op->args[k++]));
> }
> switch (c) {
> case INDEX_op_brcond_i32:
> @@ -1128,10 +1126,11 @@ void tcg_dump_ops(TCGContext *s)
> case INDEX_op_brcond_i64:
> case INDEX_op_setcond_i64:
> case INDEX_op_movcond_i64:
> - if (args[k] < ARRAY_SIZE(cond_name) && cond_name[args[k]]) {
> - col += qemu_log(",%s", cond_name[args[k++]]);
> + if (op->args[k] < ARRAY_SIZE(cond_name)
> + && cond_name[op->args[k]]) {
> + col += qemu_log(",%s", cond_name[op->args[k++]]);
> } else {
> - col += qemu_log(",$0x%" TCG_PRIlx, args[k++]);
> + col += qemu_log(",$0x%" TCG_PRIlx, op->args[k++]);
> }
> i = 1;
> break;
> @@ -1140,7 +1139,7 @@ void tcg_dump_ops(TCGContext *s)
> case INDEX_op_qemu_ld_i64:
> case INDEX_op_qemu_st_i64:
> {
> - TCGMemOpIdx oi = args[k++];
> + TCGMemOpIdx oi = op->args[k++];
> TCGMemOp op = get_memop(oi);
> unsigned ix = get_mmuidx(oi);
>
> @@ -1165,14 +1164,15 @@ void tcg_dump_ops(TCGContext *s)
> case INDEX_op_brcond_i32:
> case INDEX_op_brcond_i64:
> case INDEX_op_brcond2_i32:
> - col += qemu_log("%s$L%d", k ? "," : "", arg_label(args[k])->id);
> + col += qemu_log("%s$L%d", k ? "," : "",
> + arg_label(op->args[k])->id);
> i++, k++;
> break;
> default:
> break;
> }
> for (; i < nb_cargs; i++, k++) {
> - col += qemu_log("%s$0x%" TCG_PRIlx, k ? "," : "", args[k]);
> + col += qemu_log("%s$0x%" TCG_PRIlx, k ? "," : "", op->args[k]);
> }
> }
> if (op->life) {
> @@ -1433,7 +1433,6 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> TCGArg arg;
>
> TCGOp * const op = &s->gen_op_buf[oi];
> - TCGArg * const args = op->args;
> TCGOpcode opc = op->opc;
> const TCGOpDef *def = &tcg_op_defs[opc];
>
> @@ -1446,12 +1445,12 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
>
> nb_oargs = op->callo;
> nb_iargs = op->calli;
> - call_flags = args[nb_oargs + nb_iargs + 1];
> + call_flags = op->args[nb_oargs + nb_iargs + 1];
>
> /* pure functions can be removed if their result is unused */
> if (call_flags & TCG_CALL_NO_SIDE_EFFECTS) {
> for (i = 0; i < nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (temp_state[arg] != TS_DEAD) {
> goto do_not_remove_call;
> }
> @@ -1462,7 +1461,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
>
> /* output args are dead */
> for (i = 0; i < nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (temp_state[arg] & TS_DEAD) {
> arg_life |= DEAD_ARG << i;
> }
> @@ -1485,7 +1484,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
>
> /* record arguments that die in this helper */
> for (i = nb_oargs; i < nb_iargs + nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (arg != TCG_CALL_DUMMY_ARG) {
> if (temp_state[arg] & TS_DEAD) {
> arg_life |= DEAD_ARG << i;
> @@ -1494,7 +1493,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> }
> /* input arguments are live for preceding opcodes */
> for (i = nb_oargs; i < nb_iargs + nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (arg != TCG_CALL_DUMMY_ARG) {
> temp_state[arg] &= ~TS_DEAD;
> }
> @@ -1506,7 +1505,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> break;
> case INDEX_op_discard:
> /* mark the temporary as dead */
> - temp_state[args[0]] = TS_DEAD;
> + temp_state[op->args[0]] = TS_DEAD;
> break;
>
> case INDEX_op_add2_i32:
> @@ -1527,15 +1526,15 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> the low part. The result can be optimized to a simple
> add or sub. This happens often for x86_64 guest when the
> cpu mode is set to 32 bit. */
> - if (temp_state[args[1]] == TS_DEAD) {
> - if (temp_state[args[0]] == TS_DEAD) {
> + if (temp_state[op->args[1]] == TS_DEAD) {
> + if (temp_state[op->args[0]] == TS_DEAD) {
> goto do_remove;
> }
> /* Replace the opcode and adjust the args in place,
> leaving 3 unused args at the end. */
> op->opc = opc = opc_new;
> - args[1] = args[2];
> - args[2] = args[4];
> + op->args[1] = op->args[2];
> + op->args[2] = op->args[4];
> /* Fall through and mark the single-word operation live. */
> nb_iargs = 2;
> nb_oargs = 1;
> @@ -1565,21 +1564,21 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> do_mul2:
> nb_iargs = 2;
> nb_oargs = 2;
> - if (temp_state[args[1]] == TS_DEAD) {
> - if (temp_state[args[0]] == TS_DEAD) {
> + if (temp_state[op->args[1]] == TS_DEAD) {
> + if (temp_state[op->args[0]] == TS_DEAD) {
> /* Both parts of the operation are dead. */
> goto do_remove;
> }
> /* The high part of the operation is dead; generate the low. */
> op->opc = opc = opc_new;
> - args[1] = args[2];
> - args[2] = args[3];
> - } else if (temp_state[args[0]] == TS_DEAD && have_opc_new2) {
> + op->args[1] = op->args[2];
> + op->args[2] = op->args[3];
> + } else if (temp_state[op->args[0]] == TS_DEAD && have_opc_new2) {
> /* The low part of the operation is dead; generate the high. */
> op->opc = opc = opc_new2;
> - args[0] = args[1];
> - args[1] = args[2];
> - args[2] = args[3];
> + op->args[0] = op->args[1];
> + op->args[1] = op->args[2];
> + op->args[2] = op->args[3];
> } else {
> goto do_not_remove;
> }
> @@ -1597,7 +1596,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> implies side effects */
> if (!(def->flags & TCG_OPF_SIDE_EFFECTS) && nb_oargs != 0) {
> for (i = 0; i < nb_oargs; i++) {
> - if (temp_state[args[i]] != TS_DEAD) {
> + if (temp_state[op->args[i]] != TS_DEAD) {
> goto do_not_remove;
> }
> }
> @@ -1607,7 +1606,7 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
> do_not_remove:
> /* output args are dead */
> for (i = 0; i < nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (temp_state[arg] & TS_DEAD) {
> arg_life |= DEAD_ARG << i;
> }
> @@ -1629,14 +1628,14 @@ static void liveness_pass_1(TCGContext *s, uint8_t *temp_state)
>
> /* record arguments that die in this opcode */
> for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (temp_state[arg] & TS_DEAD) {
> arg_life |= DEAD_ARG << i;
> }
> }
> /* input arguments are live for preceding opcodes */
> for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
> - temp_state[args[i]] &= ~TS_DEAD;
> + temp_state[op->args[i]] &= ~TS_DEAD;
> }
> }
> break;
> @@ -1671,7 +1670,6 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
>
> for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
> TCGOp *op = &s->gen_op_buf[oi];
> - TCGArg *args = op->args;
> TCGOpcode opc = op->opc;
> const TCGOpDef *def = &tcg_op_defs[opc];
> TCGLifeData arg_life = op->life;
> @@ -1683,7 +1681,7 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
> if (opc == INDEX_op_call) {
> nb_oargs = op->callo;
> nb_iargs = op->calli;
> - call_flags = args[nb_oargs + nb_iargs + 1];
> + call_flags = op->args[nb_oargs + nb_iargs + 1];
> } else {
> nb_iargs = def->nb_iargs;
> nb_oargs = def->nb_oargs;
> @@ -1704,7 +1702,7 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
>
> /* Make sure that input arguments are available. */
> for (i = nb_oargs; i < nb_iargs + nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> /* Note this unsigned test catches TCG_CALL_ARG_DUMMY too. */
> if (arg < nb_globals) {
> dir = dir_temps[arg];
> @@ -1714,11 +1712,10 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
> ? INDEX_op_ld_i32
> : INDEX_op_ld_i64);
> TCGOp *lop = tcg_op_insert_before(s, op, lopc, 3);
> - TCGArg *largs = lop->args;
>
> - largs[0] = dir;
> - largs[1] = temp_idx(s, its->mem_base);
> - largs[2] = its->mem_offset;
> + lop->args[0] = dir;
> + lop->args[1] = temp_idx(s, its->mem_base);
> + lop->args[2] = its->mem_offset;
>
> /* Loaded, but synced with memory. */
> temp_state[arg] = TS_MEM;
> @@ -1730,11 +1727,11 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
> No action is required except keeping temp_state up to date
> so that we reload when needed. */
> for (i = nb_oargs; i < nb_iargs + nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (arg < nb_globals) {
> dir = dir_temps[arg];
> if (dir != 0) {
> - args[i] = dir;
> + op->args[i] = dir;
> changes = true;
> if (IS_DEAD_ARG(i)) {
> temp_state[arg] = TS_DEAD;
> @@ -1765,7 +1762,7 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
>
> /* Outputs become available. */
> for (i = 0; i < nb_oargs; i++) {
> - arg = args[i];
> + arg = op->args[i];
> if (arg >= nb_globals) {
> continue;
> }
> @@ -1773,7 +1770,7 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
> if (dir == 0) {
> continue;
> }
> - args[i] = dir;
> + op->args[i] = dir;
> changes = true;
>
> /* The output is now live and modified. */
> @@ -1786,11 +1783,10 @@ static bool liveness_pass_2(TCGContext *s, uint8_t *temp_state)
> ? INDEX_op_st_i32
> : INDEX_op_st_i64);
> TCGOp *sop = tcg_op_insert_after(s, op, sopc, 3);
> - TCGArg *sargs = sop->args;
>
> - sargs[0] = dir;
> - sargs[1] = temp_idx(s, its->mem_base);
> - sargs[2] = its->mem_offset;
> + sop->args[0] = dir;
> + sop->args[1] = temp_idx(s, its->mem_base);
> + sop->args[2] = its->mem_offset;
>
> temp_state[arg] = TS_MEM;
> }
> @@ -2614,7 +2610,6 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> num_insns = -1;
> for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
> TCGOp * const op = &s->gen_op_buf[oi];
> - TCGArg * const args = op->args;
> TCGOpcode opc = op->opc;
> const TCGOpDef *def = &tcg_op_defs[opc];
> TCGLifeData arg_life = op->life;
> @@ -2627,11 +2622,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> switch (opc) {
> case INDEX_op_mov_i32:
> case INDEX_op_mov_i64:
> - tcg_reg_alloc_mov(s, def, args, arg_life);
> + tcg_reg_alloc_mov(s, def, op->args, arg_life);
> break;
> case INDEX_op_movi_i32:
> case INDEX_op_movi_i64:
> - tcg_reg_alloc_movi(s, args, arg_life);
> + tcg_reg_alloc_movi(s, op->args, arg_life);
> break;
> case INDEX_op_insn_start:
> if (num_insns >= 0) {
> @@ -2641,22 +2636,22 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
> target_ulong a;
> #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
> - a = ((target_ulong)args[i * 2 + 1] << 32) | args[i * 2];
> + a = deposit64(op->args[i * 2], 32, 32, op->args[i * 2 + 1]);
> #else
> - a = args[i];
> + a = op->args[i];
> #endif
> s->gen_insn_data[num_insns][i] = a;
> }
> break;
> case INDEX_op_discard:
> - temp_dead(s, &s->temps[args[0]]);
> + temp_dead(s, &s->temps[op->args[0]]);
> break;
> case INDEX_op_set_label:
> tcg_reg_alloc_bb_end(s, s->reserved_regs);
> - tcg_out_label(s, arg_label(args[0]), s->code_ptr);
> + tcg_out_label(s, arg_label(op->args[0]), s->code_ptr);
> break;
> case INDEX_op_call:
> - tcg_reg_alloc_call(s, op->callo, op->calli, args, arg_life);
> + tcg_reg_alloc_call(s, op->callo, op->calli, op->args, arg_life);
> break;
> default:
> /* Sanity check that we've not introduced any unhandled opcodes. */
> @@ -2666,7 +2661,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> /* Note: in order to speed up the code, it would be much
> faster to have specialized register allocator functions for
> some common argument patterns */
> - tcg_reg_alloc_op(s, def, opc, args, arg_life);
> + tcg_reg_alloc_op(s, def, opc, op->args, arg_life);
> break;
> }
> #ifdef CONFIG_DEBUG_TCG
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
next prev parent reply other threads:[~2017-06-26 15:01 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-21 2:48 [Qemu-devel] [PATCH 00/16] Cleanups within TCG middle-end Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 01/16] tcg: Merge opcode arguments into TCGOp Richard Henderson
2017-06-26 14:44 ` Alex Bennée
2017-06-26 14:55 ` Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 02/16] tcg: Propagate args to op->args in optimizer Richard Henderson
2017-06-26 14:53 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 03/16] tcg: Propagate args to op->args in tcg.c Richard Henderson
2017-06-26 15:02 ` Alex Bennée [this message]
2017-06-26 15:07 ` Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 04/16] tcg: Propagate TCGOp down to allocators Richard Henderson
2017-06-26 15:08 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 05/16] tcg: Introduce arg_temp Richard Henderson
2017-06-26 16:37 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 06/16] tcg: Add temp_global bit to TCGTemp Richard Henderson
2017-06-27 8:39 ` Alex Bennée
2017-06-27 16:17 ` Richard Henderson
2017-06-28 8:52 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 07/16] tcg: Return NULL temp for TCG_CALL_DUMMY_ARG Richard Henderson
2017-06-27 8:47 ` Alex Bennée
2017-06-27 16:36 ` Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 08/16] tcg: Introduce temp_arg Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 09/16] tcg: Use per-temp state data in liveness Richard Henderson
2017-06-27 8:57 ` Alex Bennée
2017-06-27 16:39 ` Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 10/16] tcg: Avoid loops against variable bounds Richard Henderson
2017-06-27 9:01 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 11/16] tcg: Change temp_allocate_frame arg to TCGTemp Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 12/16] tcg: Remove unused TCG_CALL_DUMMY_TCGV Richard Henderson
2017-06-27 9:42 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 13/16] tcg: Export temp_idx Richard Henderson
2017-06-27 9:46 ` Alex Bennée
2017-06-27 16:43 ` Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 14/16] tcg: Use per-temp state data in optimize Richard Henderson
2017-06-27 9:59 ` Alex Bennée
2017-06-21 2:48 ` [Qemu-devel] [PATCH 15/16] tcg: Define separate structures for TCGv_* Richard Henderson
2017-06-21 2:48 ` [Qemu-devel] [PATCH 16/16] tcg: Store pointers to temporaries directly in TCGArg Richard Henderson
2017-06-21 3:43 ` [Qemu-devel] [PATCH 00/16] Cleanups within TCG middle-end no-reply
2017-06-26 16:49 ` Alex Bennée
2017-06-26 17:47 ` Richard Henderson
2017-06-26 19:19 ` Alex Bennée
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=8760fiyeps.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=aurelien@aurel32.net \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/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).