On 2015-07-29 17:01, Alex Bennée wrote:
>
> Aurelien Jarno <aurel...@aurel32.net> 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 <r...@twiddle.net>
> > Signed-off-by: Aurelien Jarno <aurel...@aurel32.net>
> > ---
> > 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
That's something that can be added, but in that case we need both
temp_is_const and temp_is_const_val as we sometimes need to test if
a temp is a constant without necessarily checking for a particular
value.
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net
