On 2016-06-23 20:48, Richard Henderson wrote:
> Instead of using -1 as end of chain, use 0, and link through the 0
> entry as a fully circular double-linked list.
>
> Signed-off-by: Richard Henderson <r...@twiddle.net>
> ---
> include/exec/gen-icount.h | 2 +-
> tcg/optimize.c | 8 ++------
> tcg/tcg-op.c | 2 +-
> tcg/tcg.c | 32 ++++++++++++--------------------
> tcg/tcg.h | 20 ++++++++++++--------
> 5 files changed, 28 insertions(+), 36 deletions(-)
>
> diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
> index a011324..5f16077 100644
> --- a/include/exec/gen-icount.h
> +++ b/include/exec/gen-icount.h
> @@ -59,7 +59,7 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns)
> }
>
> /* Terminate the linked list. */
> - tcg_ctx.gen_op_buf[tcg_ctx.gen_last_op_idx].next = -1;
> + tcg_ctx.gen_op_buf[tcg_ctx.gen_op_buf[0].prev].next = 0;
> }
>
> static inline void gen_io_start(void)
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index c0d975b..8df7fc7 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -103,11 +103,7 @@ static TCGOp *insert_op_before(TCGContext *s, TCGOp
> *old_op,
> .prev = prev,
> .next = next
> };
> - if (prev >= 0) {
> - s->gen_op_buf[prev].next = oi;
> - } else {
> - s->gen_first_op_idx = oi;
> - }
> + s->gen_op_buf[prev].next = oi;
> old_op->prev = oi;
>
> return new_op;
> @@ -583,7 +579,7 @@ void tcg_optimize(TCGContext *s)
> nb_globals = s->nb_globals;
> reset_all_temps(nb_temps);
>
> - for (oi = s->gen_first_op_idx; oi >= 0; oi = oi_next) {
> + for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
> tcg_target_ulong mask, partmask, affected;
> int nb_oargs, nb_iargs, i;
> TCGArg tmp;
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index 569cdc6..62d91b4 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -52,7 +52,7 @@ static void tcg_emit_op(TCGContext *ctx, TCGOpcode opc, int
> args)
> int pi = oi - 1;
>
> tcg_debug_assert(oi < OPC_BUF_SIZE);
> - ctx->gen_last_op_idx = oi;
> + ctx->gen_op_buf[0].prev = oi;
> ctx->gen_next_op_idx = ni;
>
> ctx->gen_op_buf[oi] = (TCGOp){
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 400e69c..bb1efe2 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -437,9 +437,9 @@ void tcg_func_start(TCGContext *s)
> s->goto_tb_issue_mask = 0;
> #endif
>
> - s->gen_first_op_idx = 0;
> - s->gen_last_op_idx = -1;
> - s->gen_next_op_idx = 0;
> + s->gen_op_buf[0].next = 1;
> + s->gen_op_buf[0].prev = 0;
> + s->gen_next_op_idx = 1;
> s->gen_next_parm_idx = 0;
>
> s->be = tcg_malloc(sizeof(TCGBackendData));
> @@ -868,7 +868,7 @@ void tcg_gen_callN(TCGContext *s, void *func, TCGArg ret,
> /* Make sure the calli field didn't overflow. */
> tcg_debug_assert(s->gen_op_buf[i].calli == real_args);
>
> - s->gen_last_op_idx = i;
> + s->gen_op_buf[0].prev = i;
> s->gen_next_op_idx = i + 1;
> s->gen_next_parm_idx = pi;
>
> @@ -1004,7 +1004,7 @@ void tcg_dump_ops(TCGContext *s)
> TCGOp *op;
> int oi;
>
> - for (oi = s->gen_first_op_idx; oi >= 0; oi = op->next) {
> + 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;
> @@ -1016,7 +1016,7 @@ void tcg_dump_ops(TCGContext *s)
> args = &s->gen_opparam_buf[op->args];
>
> if (c == INDEX_op_insn_start) {
> - qemu_log("%s ----", oi != s->gen_first_op_idx ? "\n" : "");
> + qemu_log("%s ----", oi != s->gen_op_buf[0].next ? "\n" : "");
>
> for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
> target_ulong a;
> @@ -1287,18 +1287,10 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
> int next = op->next;
> int prev = op->prev;
>
> - if (next >= 0) {
> - s->gen_op_buf[next].prev = prev;
> - } else {
> - s->gen_last_op_idx = prev;
> - }
> - if (prev >= 0) {
> - s->gen_op_buf[prev].next = next;
> - } else {
> - s->gen_first_op_idx = next;
> - }
> + s->gen_op_buf[next].prev = prev;
> + s->gen_op_buf[prev].next = next;
>
> - memset(op, -1, sizeof(*op));
> + memset(op, 0, sizeof(*op));
Doing so means you can remove the op at gen_op_buf[0]. The doubled
linked list is still valid, but then I guess you can't assume anymore
that the first op is gen_op_buf[0], as it is done elsewhere your patch.
I guess it's unlikely to happen that we have to remove the first op.
Maybe adding an assert there is enough?
> #ifdef CONFIG_PROFILER
> s->del_op_count++;
> @@ -1344,7 +1336,7 @@ static void tcg_liveness_analysis(TCGContext *s)
> mem_temps = tcg_malloc(s->nb_temps);
> tcg_la_func_end(s, dead_temps, mem_temps);
>
> - for (oi = s->gen_last_op_idx; oi >= 0; oi = oi_prev) {
> + for (oi = s->gen_op_buf[0].prev; oi != 0; oi = oi_prev) {
> int i, nb_iargs, nb_oargs;
> TCGOpcode opc_new, opc_new2;
> bool have_opc_new2;
> @@ -2321,7 +2313,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> {
> int n;
>
> - n = s->gen_last_op_idx + 1;
> + n = s->gen_op_buf[0].prev + 1;
> s->op_count += n;
> if (n > s->op_count_max) {
> s->op_count_max = n;
> @@ -2380,7 +2372,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
> tcg_out_tb_init(s);
>
> num_insns = -1;
> - for (oi = s->gen_first_op_idx; oi >= 0; oi = oi_next) {
> + for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
> TCGOp * const op = &s->gen_op_buf[oi];
> TCGArg * const args = &s->gen_opparam_buf[op->args];
> TCGOpcode opc = op->opc;
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index cc14560..49b396d 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -520,17 +520,21 @@ typedef struct TCGOp {
> unsigned callo : 2;
> unsigned calli : 6;
>
> - /* Index of the arguments for this op, or -1 for zero-operand ops. */
> - signed args : 16;
> + /* Index of the arguments for this op, or 0 for zero-operand ops. */
> + unsigned args : 16;
>
> - /* Index of the prex/next op, or -1 for the end of the list. */
> - signed prev : 16;
> - signed next : 16;
> + /* Index of the prex/next op, or 0 for the end of the list. */
It's not introduced by your patch, but you might want to fix the typo
prex -> prev.
> + unsigned prev : 16;
> + unsigned next : 16;
> } TCGOp;
>
> -QEMU_BUILD_BUG_ON(NB_OPS > 0xff);
> -QEMU_BUILD_BUG_ON(OPC_BUF_SIZE >= 0x7fff);
> -QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE >= 0x7fff);
> +/* Make sure operands fit in the bitfields above. */
> +QEMU_BUILD_BUG_ON(NB_OPS > (1 << 8));
> +QEMU_BUILD_BUG_ON(OPC_BUF_SIZE > (1 << 16));
> +QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 16));
> +
> +/* Make sure that we don't overflow 64 bits without noticing. */
> +QEMU_BUILD_BUG_ON(sizeof(TCGOp) > 8);
>
> struct TCGContext {
> uint8_t *pool_cur, *pool_end;
It seems that gen_first_op_idx and gen_last_op_idx are now unused.
Shouldn't they be removed?
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net
