Re: [Qemu-ppc] [Qemu-devel] [PATCH] PPC: Depend behavior of cmp instructions only on instruction encoding

[Aurelien Jarno]

On Wed, May 08, 2013 at 03:26:00PM +0200, Alexander Graf wrote:
> When running an L=1 cmp instruction on a 64bit PPC CPU with SF off, it
> still behaves identical to what it does when SF is on. Remove the implicit
> difference in the code.
> 
> However, the situation is more complex than that. On 32bit CPUs, L=1
> instructions are either treated identical to L=0 instructions (G4) or
> treated as illegal instructions (e500mc). Differenciating these cases
> is out of scope for the 1.5 release and will follow afterwards. For now
> just treat the 32bit CPU, 64bit cmp case as undefined.
> 
> Reported-by: Torbjorn Granlund <address@hidden>
> Signed-off-by: Alexander Graf <address@hidden>
> ---
>  target-ppc/translate.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a018616..89a4445 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -675,7 +675,7 @@ static inline void gen_set_Rc0(DisasContext *ctx, TCGv 
> reg)
>  /* cmp */
>  static void gen_cmp(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +    if (!(ctx->opcode & 0x00200000)) {
>          gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                       1, crfD(ctx->opcode));
>      } else {
> @@ -687,7 +687,7 @@ static void gen_cmp(DisasContext *ctx)
>  /* cmpi */
>  static void gen_cmpi(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +    if (!(ctx->opcode & 0x00200000)) {
>          gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], SIMM(ctx->opcode),
>                        1, crfD(ctx->opcode));
>      } else {
> @@ -699,7 +699,7 @@ static void gen_cmpi(DisasContext *ctx)
>  /* cmpl */
>  static void gen_cmpl(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +    if (!(ctx->opcode & 0x00200000)) {
>          gen_op_cmp32(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],
>                       0, crfD(ctx->opcode));
>      } else {
> @@ -711,7 +711,7 @@ static void gen_cmpl(DisasContext *ctx)
>  /* cmpli */
>  static void gen_cmpli(DisasContext *ctx)
>  {
> -    if (NARROW_MODE(ctx) || !(ctx->opcode & 0x00200000)) {
> +    if (!(ctx->opcode & 0x00200000)) {
>          gen_op_cmpi32(cpu_gpr[rA(ctx->opcode)], UIMM(ctx->opcode),
>                        0, crfD(ctx->opcode));
>      } else {

Reviewed-by: Aurelien Jarno <address@hidden>

That said this does implement neither the specification nor the silicon
behaviour. This is fine for 1.5 as we are in freeze period, but this 
should be fixed for the 1.6 release.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net