From: Aurelien Jarno <aurelien@aurel32.net>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] ppc: Convert op_andi to TCG
Date: Sun, 14 Sep 2008 20:34:10 +0200 [thread overview]
Message-ID: <20080914183410.GJ22422@volta.aurel32.net> (raw)
In-Reply-To: <AA914B55-BA79-4F79-8AFF-275CFD316896@web.de>
On Sun, Sep 14, 2008 at 01:23:42PM +0200, Andreas Färber wrote:
> Replace op_andi_... with tcg_gen_andi_tl.
>
> Signed-off-by: Andreas Faerber <andreas.faerber@web.de>
Applied, thanks.
> ---
> This patch reveals a problem with the crf conversion to i32:
> As you can see we now have a mix of _tl and _i32 for cpu_T[].
> I would suggest to convert cpu_crf to tl to avoid problems
> (it was converted from i8 to i32 previously).
While I agree there is a problem, I disagree about the solution. cpu_T[]
registers will eventually disappear. Changing things to match them is a
bad idea. The solution is to get rid of them in functions manipulating
cpu_crf. I'll commit a patch later to fix that.
> target-ppc/op.c | 27 ---------------------------
> target-ppc/translate.c | 38 +++++++++++---------------------------
> 2 files changed, 11 insertions(+), 54 deletions(-)
>
> diff --git a/target-ppc/op.c b/target-ppc/op.c
> index 95ab8b9..6416659 100644
> --- a/target-ppc/op.c
> +++ b/target-ppc/op.c
> @@ -1153,33 +1153,6 @@ void OPPROTO op_andc (void)
> RETURN();
> }
>
> -/* andi. */
> -void OPPROTO op_andi_T0 (void)
> -{
> - T0 &= (uint32_t)PARAM1;
> - RETURN();
> -}
> -
> -void OPPROTO op_andi_T1 (void)
> -{
> - T1 &= (uint32_t)PARAM1;
> - RETURN();
> -}
> -
> -#if defined(TARGET_PPC64)
> -void OPPROTO op_andi_T0_64 (void)
> -{
> - T0 &= ((uint64_t)PARAM1 << 32) | (uint64_t)PARAM2;
> - RETURN();
> -}
> -
> -void OPPROTO op_andi_T1_64 (void)
> -{
> - T1 &= ((uint64_t)PARAM1 << 32) | (uint64_t)PARAM2;
> - RETURN();
> -}
> -#endif
> -
> /* count leading zero */
> void OPPROTO op_cntlzw (void)
> {
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index cc1f836..56bdef2 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1235,7 +1235,7 @@ GEN_LOGICAL2(andc, 0x01, PPC_INTEGER);
> GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER)
> {
> tcg_gen_mov_tl(cpu_T[0], cpu_gpr[rS(ctx->opcode)]);
> - gen_op_andi_T0(UIMM(ctx->opcode));
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], UIMM(ctx->opcode));
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> gen_set_Rc0(ctx);
> }
> @@ -1243,7 +1243,7 @@ GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF,
> 0x00000000, PPC_INTEGER)
> GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000,
> PPC_INTEGER)
> {
> tcg_gen_mov_tl(cpu_T[0], cpu_gpr[rS(ctx->opcode)]);
> - gen_op_andi_T0(UIMM(ctx->opcode) << 16);
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], UIMM(ctx->opcode) << 16);
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> gen_set_Rc0(ctx);
> }
> @@ -1458,8 +1458,8 @@ GEN_HANDLER(rlwimi, 0x14, 0xFF, 0xFF, 0x00000000,
> PPC_INTEGER)
> me += 32;
> #endif
> mask = MASK(mb, me);
> - gen_op_andi_T0(mask);
> - gen_op_andi_T1(~mask);
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], mask);
> + tcg_gen_andi_tl(cpu_T[1], cpu_T[1], ~mask);
> gen_op_or();
> do_store:
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> @@ -1498,7 +1498,7 @@ GEN_HANDLER(rlwinm, 0x15, 0xFF, 0xFF, 0x00000000,
> PPC_INTEGER)
> mb += 32;
> me += 32;
> #endif
> - gen_op_andi_T0(MASK(mb, me));
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
> do_store:
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1519,7 +1519,7 @@ GEN_HANDLER(rlwnm, 0x17, 0xFF, 0xFF, 0x00000000,
> PPC_INTEGER)
> mb += 32;
> me += 32;
> #endif
> - gen_op_andi_T0(MASK(mb, me));
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
> }
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1558,22 +1558,6 @@ GEN_HANDLER2(name##3, stringify(name), opc1, opc2
> | 0x11, 0xFF, 0x00000000, \
> gen_##name(ctx, 1, 1);
> \
> }
>
> -static always_inline void gen_andi_T0_64 (DisasContext *ctx, uint64_t
> mask)
> -{
> - if (mask >> 32)
> - gen_op_andi_T0_64(mask >> 32, mask & 0xFFFFFFFF);
> - else
> - gen_op_andi_T0(mask);
> -}
> -
> -static always_inline void gen_andi_T1_64 (DisasContext *ctx, uint64_t
> mask)
> -{
> - if (mask >> 32)
> - gen_op_andi_T1_64(mask >> 32, mask & 0xFFFFFFFF);
> - else
> - gen_op_andi_T1(mask);
> -}
> -
> static always_inline void gen_rldinm (DisasContext *ctx, uint32_t mb,
> uint32_t me, uint32_t sh)
> {
> @@ -1597,7 +1581,7 @@ static always_inline void gen_rldinm (DisasContext
> *ctx, uint32_t mb,
> }
> gen_op_rotli64_T0(sh);
> do_mask:
> - gen_andi_T0_64(ctx, MASK(mb, me));
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
> do_store:
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1641,7 +1625,7 @@ static always_inline void gen_rldnm (DisasContext
> *ctx, uint32_t mb,
> tcg_gen_mov_tl(cpu_T[1], cpu_gpr[rB(ctx->opcode)]);
> gen_op_rotl64_T0_T1();
> if (unlikely(mb != 0 || me != 63)) {
> - gen_andi_T0_64(ctx, MASK(mb, me));
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], MASK(mb, me));
> }
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> if (unlikely(Rc(ctx->opcode) != 0))
> @@ -1689,8 +1673,8 @@ static always_inline void gen_rldimi (DisasContext
> *ctx, int mbn, int shn)
> gen_op_rotli64_T0(sh);
> do_mask:
> mask = MASK(mb, me);
> - gen_andi_T0_64(ctx, mask);
> - gen_andi_T1_64(ctx, ~mask);
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], mask);
> + tcg_gen_andi_tl(cpu_T[1], cpu_T[1], ~mask);
> gen_op_or();
> do_store:
> tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], cpu_T[0]);
> @@ -3107,7 +3091,7 @@ GEN_HANDLER(cr##op, 0x13, 0x01, opc, 0x00000001,
> PPC_INTEGER) \
> gen_op_sli_T1(-sh);
> \
> gen_op_##op();
> \
> bitmask = 1 << (3 - (crbD(ctx->opcode) & 0x03));
> \
> - gen_op_andi_T0(bitmask);
> \
> + tcg_gen_andi_tl(cpu_T[0], cpu_T[0], bitmask);
> \
> tcg_gen_andi_i32(cpu_T[1], cpu_crf[crbD(ctx->opcode) >> 2],
> ~bitmask); \
> gen_op_or();
> \
> tcg_gen_andi_i32(cpu_crf[crbD(ctx->opcode) >> 2], cpu_T[0], 0xf);
> \
> --
> 1.5.5.1
>
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' aurel32@debian.org | aurelien@aurel32.net
`- people.debian.org/~aurel32 | www.aurel32.net
next prev parent reply other threads:[~2008-09-14 18:34 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-14 11:06 [Qemu-devel] [PATCH 1/2] ppc: Convert ctr, lr moves to TCG Andreas Färber
2008-09-14 11:23 ` [Qemu-devel] [PATCH 2/2] ppc: Convert op_andi " Andreas Färber
2008-09-14 11:35 ` Blue Swirl
2008-09-14 11:48 ` Andreas Färber
2008-09-14 11:53 ` Blue Swirl
2008-09-14 12:02 ` Andreas Färber
2008-09-14 12:03 ` Paul Brook
2008-09-14 12:13 ` Blue Swirl
2008-09-14 18:34 ` Aurelien Jarno [this message]
2008-09-14 18:30 ` [Qemu-devel] [PATCH 1/2] ppc: Convert ctr, lr moves " Aurelien Jarno
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=20080914183410.GJ22422@volta.aurel32.net \
--to=aurelien@aurel32.net \
--cc=qemu-devel@nongnu.org \
/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).