[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.5 02/10] tcg/optimize: add temp_is_const a
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH for-2.5 02/10] tcg/optimize: add temp_is_const and temp_is_copy functions |
Date: |
Wed, 29 Jul 2015 17:01:00 +0100 |
Aurelien Jarno <address@hidden> writes:
> Add two accessor functions temp_is_const and temp_is_copy, to make the
> code more readable and make code change easier.
>
> Cc: Richard Henderson <address@hidden>
> Signed-off-by: Aurelien Jarno <address@hidden>
> ---
> tcg/optimize.c | 131
> ++++++++++++++++++++++++++-------------------------------
> 1 file changed, 60 insertions(+), 71 deletions(-)
>
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index 20e24b3..d2b63a4 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -52,11 +52,21 @@ struct tcg_temp_info {
> static struct tcg_temp_info temps[TCG_MAX_TEMPS];
> static TCGTempSet temps_used;
>
> +static inline bool temp_is_const(TCGArg arg)
> +{
> + return temps[arg].state == TCG_TEMP_CONST;
> +}
> +
> +static inline bool temp_is_copy(TCGArg arg)
> +{
> + return temps[arg].state == TCG_TEMP_COPY;
> +}
> +
> /* Reset TEMP's state to TCG_TEMP_UNDEF. If TEMP only had one copy, remove
> the copy flag from the left temp. */
> static void reset_temp(TCGArg temp)
> {
> - if (temps[temp].state == TCG_TEMP_COPY) {
> + if (temp_is_copy(temp)) {
> if (temps[temp].prev_copy == temps[temp].next_copy) {
> temps[temps[temp].next_copy].state = TCG_TEMP_UNDEF;
> } else {
> @@ -186,8 +196,7 @@ static bool temps_are_copies(TCGArg arg1, TCGArg arg2)
> return true;
> }
>
> - if (temps[arg1].state != TCG_TEMP_COPY
> - || temps[arg2].state != TCG_TEMP_COPY) {
> + if (!temp_is_copy(arg1) || !temp_is_copy(arg2)) {
> return false;
> }
>
> @@ -230,7 +239,7 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op,
> TCGArg *args,
> return;
> }
>
> - if (temps[src].state == TCG_TEMP_CONST) {
> + if (temp_is_const(src)) {
> tcg_opt_gen_movi(s, op, args, dst, temps[src].val);
> return;
> }
> @@ -248,10 +257,10 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *op,
> TCGArg *args,
> }
> temps[dst].mask = mask;
>
> - assert(temps[src].state != TCG_TEMP_CONST);
> + assert(!temp_is_const(src));
>
> if (s->temps[src].type == s->temps[dst].type) {
> - if (temps[src].state != TCG_TEMP_COPY) {
> + if (!temp_is_copy(src)) {
> temps[src].state = TCG_TEMP_COPY;
> temps[src].next_copy = src;
> temps[src].prev_copy = src;
> @@ -488,7 +497,7 @@ static bool do_constant_folding_cond_eq(TCGCond c)
> static TCGArg do_constant_folding_cond(TCGOpcode op, TCGArg x,
> TCGArg y, TCGCond c)
> {
> - if (temps[x].state == TCG_TEMP_CONST && temps[y].state ==
> TCG_TEMP_CONST) {
> + if (temp_is_const(x) && temp_is_const(y)) {
> switch (op_bits(op)) {
> case 32:
> return do_constant_folding_cond_32(temps[x].val, temps[y].val,
> c);
> @@ -499,7 +508,7 @@ static TCGArg do_constant_folding_cond(TCGOpcode op,
> TCGArg x,
> }
> } else if (temps_are_copies(x, y)) {
> return do_constant_folding_cond_eq(c);
> - } else if (temps[y].state == TCG_TEMP_CONST && temps[y].val == 0) {
> + } else if (temp_is_const(y) && temps[y].val == 0) {
> switch (c) {
> case TCG_COND_LTU:
> return 0;
> @@ -520,12 +529,10 @@ static TCGArg do_constant_folding_cond2(TCGArg *p1,
> TCGArg *p2, TCGCond c)
> TCGArg al = p1[0], ah = p1[1];
> TCGArg bl = p2[0], bh = p2[1];
>
> - if (temps[bl].state == TCG_TEMP_CONST
> - && temps[bh].state == TCG_TEMP_CONST) {
> + if (temp_is_const(bl) && temp_is_const(bh)) {
> uint64_t b = ((uint64_t)temps[bh].val << 32) |
> (uint32_t)temps[bl].val;
>
> - if (temps[al].state == TCG_TEMP_CONST
> - && temps[ah].state == TCG_TEMP_CONST) {
> + if (temp_is_const(al) && temp_is_const(ah)) {
> uint64_t a;
> a = ((uint64_t)temps[ah].val << 32) | (uint32_t)temps[al].val;
> return do_constant_folding_cond_64(a, b, c);
> @@ -551,8 +558,8 @@ static bool swap_commutative(TCGArg dest, TCGArg *p1,
> TCGArg *p2)
> {
> TCGArg a1 = *p1, a2 = *p2;
> int sum = 0;
> - sum += temps[a1].state == TCG_TEMP_CONST;
> - sum -= temps[a2].state == TCG_TEMP_CONST;
> + sum += temp_is_const(a1);
> + sum -= temp_is_const(a2);
>
> /* Prefer the constant in second argument, and then the form
> op a, a, b, which is better handled on non-RISC hosts. */
> @@ -567,10 +574,10 @@ static bool swap_commutative(TCGArg dest, TCGArg *p1,
> TCGArg *p2)
> static bool swap_commutative2(TCGArg *p1, TCGArg *p2)
> {
> int sum = 0;
> - sum += temps[p1[0]].state == TCG_TEMP_CONST;
> - sum += temps[p1[1]].state == TCG_TEMP_CONST;
> - sum -= temps[p2[0]].state == TCG_TEMP_CONST;
> - sum -= temps[p2[1]].state == TCG_TEMP_CONST;
> + sum += temp_is_const(p1[0]);
> + sum += temp_is_const(p1[1]);
> + sum -= temp_is_const(p2[0]);
> + sum -= temp_is_const(p2[1]);
> if (sum > 0) {
> TCGArg t;
> t = p1[0], p1[0] = p2[0], p2[0] = t;
> @@ -620,7 +627,7 @@ void tcg_optimize(TCGContext *s)
>
> /* Do copy propagation */
> for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
> - if (temps[args[i]].state == TCG_TEMP_COPY) {
> + if (temp_is_copy(args[i])) {
> args[i] = find_better_copy(s, args[i]);
> }
> }
> @@ -690,8 +697,7 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(sar):
> CASE_OP_32_64(rotl):
> CASE_OP_32_64(rotr):
> - if (temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[1]].val == 0) {
> + if (temp_is_const(args[1]) && temps[args[1]].val == 0) {
> tcg_opt_gen_movi(s, op, args, args[0], 0);
> continue;
> }
> @@ -701,7 +707,7 @@ void tcg_optimize(TCGContext *s)
> TCGOpcode neg_op;
> bool have_neg;
>
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> /* Proceed with possible constant folding. */
> break;
> }
> @@ -715,8 +721,7 @@ void tcg_optimize(TCGContext *s)
> if (!have_neg) {
> break;
> }
> - if (temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[1]].val == 0) {
> + if (temp_is_const(args[1]) && temps[args[1]].val ==
> 0) {
This makes me wonder if we should have:
temp_is_const_val(arg, val)
to wrap up these tests even more neatly
> op->opc = neg_op;
> reset_temp(args[0]);
> args[1] = args[2];
> @@ -726,34 +731,30 @@ void tcg_optimize(TCGContext *s)
> break;
> CASE_OP_32_64(xor):
> CASE_OP_32_64(nand):
> - if (temps[args[1]].state != TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == -1) {
> + if (!temp_is_const(args[1])
> + && temp_is_const(args[2]) && temps[args[2]].val == -1) {
> i = 1;
> goto try_not;
> }
> break;
> CASE_OP_32_64(nor):
> - if (temps[args[1]].state != TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == 0) {
> + if (!temp_is_const(args[1])
> + && temp_is_const(args[2]) && temps[args[2]].val == 0) {
> i = 1;
> goto try_not;
> }
> break;
> CASE_OP_32_64(andc):
> - if (temps[args[2]].state != TCG_TEMP_CONST
> - && temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[1]].val == -1) {
> + if (!temp_is_const(args[2])
> + && temp_is_const(args[1]) && temps[args[1]].val == -1) {
> i = 2;
> goto try_not;
> }
> break;
> CASE_OP_32_64(orc):
> CASE_OP_32_64(eqv):
> - if (temps[args[2]].state != TCG_TEMP_CONST
> - && temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[1]].val == 0) {
> + if (!temp_is_const(args[2])
> + && temp_is_const(args[1]) && temps[args[1]].val == 0) {
> i = 2;
> goto try_not;
> }
> @@ -794,9 +795,8 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(or):
> CASE_OP_32_64(xor):
> CASE_OP_32_64(andc):
> - if (temps[args[1]].state != TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == 0) {
> + if (!temp_is_const(args[1])
> + && temp_is_const(args[2]) && temps[args[2]].val == 0) {
> tcg_opt_gen_mov(s, op, args, args[0], args[1]);
> continue;
> }
> @@ -804,9 +804,8 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(and):
> CASE_OP_32_64(orc):
> CASE_OP_32_64(eqv):
> - if (temps[args[1]].state != TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == -1) {
> + if (!temp_is_const(args[1])
> + && temp_is_const(args[2]) && temps[args[2]].val == -1) {
> tcg_opt_gen_mov(s, op, args, args[0], args[1]);
> continue;
> }
> @@ -844,7 +843,7 @@ void tcg_optimize(TCGContext *s)
>
> CASE_OP_32_64(and):
> mask = temps[args[2]].mask;
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> and_const:
> affected = temps[args[1]].mask & ~mask;
> }
> @@ -854,7 +853,7 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(andc):
> /* Known-zeros does not imply known-ones. Therefore unless
> args[2] is constant, we can't infer anything from it. */
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> mask = ~temps[args[2]].mask;
> goto and_const;
> }
> @@ -863,26 +862,26 @@ void tcg_optimize(TCGContext *s)
> break;
>
> case INDEX_op_sar_i32:
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> tmp = temps[args[2]].val & 31;
> mask = (int32_t)temps[args[1]].mask >> tmp;
> }
> break;
> case INDEX_op_sar_i64:
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> tmp = temps[args[2]].val & 63;
> mask = (int64_t)temps[args[1]].mask >> tmp;
> }
> break;
>
> case INDEX_op_shr_i32:
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> tmp = temps[args[2]].val & 31;
> mask = (uint32_t)temps[args[1]].mask >> tmp;
> }
> break;
> case INDEX_op_shr_i64:
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> tmp = temps[args[2]].val & 63;
> mask = (uint64_t)temps[args[1]].mask >> tmp;
> }
> @@ -893,7 +892,7 @@ void tcg_optimize(TCGContext *s)
> break;
>
> CASE_OP_32_64(shl):
> - if (temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2])) {
> tmp = temps[args[2]].val & (TCG_TARGET_REG_BITS - 1);
> mask = temps[args[1]].mask << tmp;
> }
> @@ -974,8 +973,7 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(mul):
> CASE_OP_32_64(muluh):
> CASE_OP_32_64(mulsh):
> - if ((temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == 0)) {
> + if ((temp_is_const(args[2]) && temps[args[2]].val == 0)) {
> tcg_opt_gen_movi(s, op, args, args[0], 0);
> continue;
> }
> @@ -1030,7 +1028,7 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(ext16u):
> case INDEX_op_ext32s_i64:
> case INDEX_op_ext32u_i64:
> - if (temps[args[1]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[1])) {
> tmp = do_constant_folding(opc, temps[args[1]].val, 0);
> tcg_opt_gen_movi(s, op, args, args[0], tmp);
> break;
> @@ -1038,7 +1036,7 @@ void tcg_optimize(TCGContext *s)
> goto do_default;
>
> case INDEX_op_trunc_shr_i32:
> - if (temps[args[1]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[1])) {
> tmp = do_constant_folding(opc, temps[args[1]].val, args[2]);
> tcg_opt_gen_movi(s, op, args, args[0], tmp);
> break;
> @@ -1067,8 +1065,7 @@ void tcg_optimize(TCGContext *s)
> CASE_OP_32_64(divu):
> CASE_OP_32_64(rem):
> CASE_OP_32_64(remu):
> - if (temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[1]) && temp_is_const(args[2])) {
> tmp = do_constant_folding(opc, temps[args[1]].val,
> temps[args[2]].val);
> tcg_opt_gen_movi(s, op, args, args[0], tmp);
> @@ -1077,8 +1074,7 @@ void tcg_optimize(TCGContext *s)
> goto do_default;
>
> CASE_OP_32_64(deposit):
> - if (temps[args[1]].state == TCG_TEMP_CONST
> - && temps[args[2]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[1]) && temp_is_const(args[2])) {
> tmp = deposit64(temps[args[1]].val, args[3], args[4],
> temps[args[2]].val);
> tcg_opt_gen_movi(s, op, args, args[0], tmp);
> @@ -1118,10 +1114,8 @@ void tcg_optimize(TCGContext *s)
>
> case INDEX_op_add2_i32:
> case INDEX_op_sub2_i32:
> - if (temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[3]].state == TCG_TEMP_CONST
> - && temps[args[4]].state == TCG_TEMP_CONST
> - && temps[args[5]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2]) && temp_is_const(args[3])
> + && temp_is_const(args[4]) && temp_is_const(args[5])) {
> uint32_t al = temps[args[2]].val;
> uint32_t ah = temps[args[3]].val;
> uint32_t bl = temps[args[4]].val;
> @@ -1150,8 +1144,7 @@ void tcg_optimize(TCGContext *s)
> goto do_default;
>
> case INDEX_op_mulu2_i32:
> - if (temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[3]].state == TCG_TEMP_CONST) {
> + if (temp_is_const(args[2]) && temp_is_const(args[3])) {
> uint32_t a = temps[args[2]].val;
> uint32_t b = temps[args[3]].val;
> uint64_t r = (uint64_t)a * b;
> @@ -1183,10 +1176,8 @@ void tcg_optimize(TCGContext *s)
> tcg_op_remove(s, op);
> }
> } else if ((args[4] == TCG_COND_LT || args[4] == TCG_COND_GE)
> - && temps[args[2]].state == TCG_TEMP_CONST
> - && temps[args[3]].state == TCG_TEMP_CONST
> - && temps[args[2]].val == 0
> - && temps[args[3]].val == 0) {
> + && temp_is_const(args[2]) && temps[args[2]].val == 0
> + && temp_is_const(args[3]) && temps[args[3]].val == 0)
> {
> /* Simplify LT/GE comparisons vs zero to a single compare
> vs the high word of the input. */
> do_brcond_high:
> @@ -1248,10 +1239,8 @@ void tcg_optimize(TCGContext *s)
> do_setcond_const:
> tcg_opt_gen_movi(s, op, args, args[0], tmp);
> } else if ((args[5] == TCG_COND_LT || args[5] == TCG_COND_GE)
> - && temps[args[3]].state == TCG_TEMP_CONST
> - && temps[args[4]].state == TCG_TEMP_CONST
> - && temps[args[3]].val == 0
> - && temps[args[4]].val == 0) {
> + && temp_is_const(args[3]) && temps[args[3]].val == 0
> + && temp_is_const(args[4]) && temps[args[4]].val == 0)
> {
> /* Simplify LT/GE comparisons vs zero to a single compare
> vs the high word of the input. */
> do_setcond_high:
Otherwise it look sane to me:
Reviewed-by: Alex Bennée <address@hidden>
--
Alex Bennée
- [Qemu-devel] [PATCH for-2.5 06/10] tcg: don't abuse TCG type in tcg_gen_trunc_shr_i64_i32, (continued)
- [Qemu-devel] [PATCH for-2.5 06/10] tcg: don't abuse TCG type in tcg_gen_trunc_shr_i64_i32, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 09/10] tcg/optimize: do not remember garbage high bits for 32-bit ops, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 04/10] tcg/optimize: allow constant to have copies, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 08/10] tcg/optimize: add optimizations for ext_i32_i64 and extu_i32_i64 ops, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 05/10] tcg: rename trunc_shr_i32 into trunc_shr_i64_i32, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 02/10] tcg/optimize: add temp_is_const and temp_is_copy functions, Aurelien Jarno, 2015/07/24
- Re: [Qemu-devel] [PATCH for-2.5 02/10] tcg/optimize: add temp_is_const and temp_is_copy functions,
Alex Bennée <=
- [Qemu-devel] [PATCH for-2.5 03/10] tcg/optimize: track const/copy status separately, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 01/10] tcg/optimize: optimize temps tracking, Aurelien Jarno, 2015/07/24
- [Qemu-devel] [PATCH for-2.5 07/10] tcg: implement real ext_i32_i64 and extu_i32_i64 ops, Aurelien Jarno, 2015/07/24