From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53807) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKUAo-0000Nq-K5 for qemu-devel@nongnu.org; Wed, 29 Jul 2015 12:25:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKUAi-0008QU-3j for qemu-devel@nongnu.org; Wed, 29 Jul 2015 12:25:42 -0400 Received: from hall.aurel32.net ([2001:bc8:30d7:100::1]:59016) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKUAh-0008Pj-Cr for qemu-devel@nongnu.org; Wed, 29 Jul 2015 12:25:36 -0400 Date: Wed, 29 Jul 2015 18:25:23 +0200 From: Aurelien Jarno Message-ID: <20150729162523.GA1603@aurel32.net> References: <1437755447-10537-1-git-send-email-aurelien@aurel32.net> <1437755447-10537-3-git-send-email-aurelien@aurel32.net> <87mvyflzr7.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <87mvyflzr7.fsf@linaro.org> Subject: Re: [Qemu-devel] [PATCH for-2.5 02/10] tcg/optimize: add temp_is_const and temp_is_copy functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex =?iso-8859-15?Q?Benn=E9e?= Cc: qemu-devel@nongnu.org, Richard Henderson On 2015-07-29 17:01, Alex Benn=E9e wrote: >=20 > Aurelien Jarno writes: >=20 > > 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 > > Signed-off-by: Aurelien Jarno > > --- > > 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; > > =20 > > +static inline bool temp_is_const(TCGArg arg) > > +{ > > + return temps[arg].state =3D=3D TCG_TEMP_CONST; > > +} > > + > > +static inline bool temp_is_copy(TCGArg arg) > > +{ > > + return temps[arg].state =3D=3D TCG_TEMP_COPY; > > +} > > + > > /* Reset TEMP's state to TCG_TEMP_UNDEF. If TEMP only had one copy, r= emove > > the copy flag from the left temp. */ > > static void reset_temp(TCGArg temp) > > { > > - if (temps[temp].state =3D=3D TCG_TEMP_COPY) { > > + if (temp_is_copy(temp)) { > > if (temps[temp].prev_copy =3D=3D temps[temp].next_copy) { > > temps[temps[temp].next_copy].state =3D TCG_TEMP_UNDEF; > > } else { > > @@ -186,8 +196,7 @@ static bool temps_are_copies(TCGArg arg1, TCGArg ar= g2) > > return true; > > } > > =20 > > - if (temps[arg1].state !=3D TCG_TEMP_COPY > > - || temps[arg2].state !=3D TCG_TEMP_COPY) { > > + if (!temp_is_copy(arg1) || !temp_is_copy(arg2)) { > > return false; > > } > > =20 > > @@ -230,7 +239,7 @@ static void tcg_opt_gen_mov(TCGContext *s, TCGOp *o= p, TCGArg *args, > > return; > > } > > =20 > > - if (temps[src].state =3D=3D 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 =3D mask; > > =20 > > - assert(temps[src].state !=3D TCG_TEMP_CONST); > > + assert(!temp_is_const(src)); > > =20 > > if (s->temps[src].type =3D=3D s->temps[dst].type) { > > - if (temps[src].state !=3D TCG_TEMP_COPY) { > > + if (!temp_is_copy(src)) { > > temps[src].state =3D TCG_TEMP_COPY; > > temps[src].next_copy =3D src; > > temps[src].prev_copy =3D 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 =3D=3D TCG_TEMP_CONST && temps[y].state =3D=3D = 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 =3D=3D TCG_TEMP_CONST && temps[y].val = =3D=3D 0) { > > + } else if (temp_is_const(y) && temps[y].val =3D=3D 0) { > > switch (c) { > > case TCG_COND_LTU: > > return 0; > > @@ -520,12 +529,10 @@ static TCGArg do_constant_folding_cond2(TCGArg *p= 1, TCGArg *p2, TCGCond c) > > TCGArg al =3D p1[0], ah =3D p1[1]; > > TCGArg bl =3D p2[0], bh =3D p2[1]; > > =20 > > - if (temps[bl].state =3D=3D TCG_TEMP_CONST > > - && temps[bh].state =3D=3D TCG_TEMP_CONST) { > > + if (temp_is_const(bl) && temp_is_const(bh)) { > > uint64_t b =3D ((uint64_t)temps[bh].val << 32) | (uint32_t)tem= ps[bl].val; > > =20 > > - if (temps[al].state =3D=3D TCG_TEMP_CONST > > - && temps[ah].state =3D=3D TCG_TEMP_CONST) { > > + if (temp_is_const(al) && temp_is_const(ah)) { > > uint64_t a; > > a =3D ((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 *p= 1, TCGArg *p2) > > { > > TCGArg a1 =3D *p1, a2 =3D *p2; > > int sum =3D 0; > > - sum +=3D temps[a1].state =3D=3D TCG_TEMP_CONST; > > - sum -=3D temps[a2].state =3D=3D TCG_TEMP_CONST; > > + sum +=3D temp_is_const(a1); > > + sum -=3D temp_is_const(a2); > > =20 > > /* 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 =3D 0; > > - sum +=3D temps[p1[0]].state =3D=3D TCG_TEMP_CONST; > > - sum +=3D temps[p1[1]].state =3D=3D TCG_TEMP_CONST; > > - sum -=3D temps[p2[0]].state =3D=3D TCG_TEMP_CONST; > > - sum -=3D temps[p2[1]].state =3D=3D TCG_TEMP_CONST; > > + sum +=3D temp_is_const(p1[0]); > > + sum +=3D temp_is_const(p1[1]); > > + sum -=3D temp_is_const(p2[0]); > > + sum -=3D temp_is_const(p2[1]); > > if (sum > 0) { > > TCGArg t; > > t =3D p1[0], p1[0] =3D p2[0], p2[0] =3D t; > > @@ -620,7 +627,7 @@ void tcg_optimize(TCGContext *s) > > =20 > > /* Do copy propagation */ > > for (i =3D nb_oargs; i < nb_oargs + nb_iargs; i++) { > > - if (temps[args[i]].state =3D=3D TCG_TEMP_COPY) { > > + if (temp_is_copy(args[i])) { > > args[i] =3D 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 =3D=3D TCG_TEMP_CONST > > - && temps[args[1]].val =3D=3D 0) { > > + if (temp_is_const(args[1]) && temps[args[1]].val =3D=3D 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; > > =20 > > - if (temps[args[2]].state =3D=3D 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 =3D=3D TCG_TEMP_CONST > > - && temps[args[1]].val =3D=3D 0) { > > + if (temp_is_const(args[1]) && temps[args[1]].val =3D=3D > > 0) { >=20 > This makes me wonder if we should have: >=20 > temp_is_const_val(arg, val) >=20 > 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. --=20 Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net