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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.