qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Some patch about mips, gen_HILO bug fix.
@ 2012-12-10  8:37 Elta Era
  2012-12-10  9:22 ` 陳韋任 (Wei-Ren Chen)
  2012-12-10 17:41 ` Richard Henderson
  0 siblings, 2 replies; 4+ messages in thread
From: Elta Era @ 2012-12-10  8:37 UTC (permalink / raw)
  To: qemu-devel


[-- Attachment #1.1: Type: text/plain, Size: 330 bytes --]

Hi all, I make three patch about mips

1: Fix my email address in dsp_helper.c
2: Fix repl_ph, value should sign-extend to target_long
3: Fix gen_HILO, there is a bug when we use dsp arch, at that time acc
index will be 0-3, and mipsdsp already add in. mipsdsp just take acc index
from opcode, on other arch, it my bring a error.

[-- Attachment #1.2: Type: text/html, Size: 375 bytes --]

[-- Attachment #2: 0001-Fix-my-email-address.patch --]
[-- Type: application/octet-stream, Size: 1052 bytes --]

From c4730313ad513081d311a65e1af2390fabbb0007 Mon Sep 17 00:00:00 2001
From: Dongxue Zhang <elta.era@gmail.com>
Date: Mon, 10 Dec 2012 16:30:03 +0800
Subject: [PATCH 1/3] Fix-my-email-address
Content-Type: text/plain; charset="utf-8"

Fix my email address from elat.era@gmail.com to elta.era@gmail.com

Signed-off-by: Dongxue Zhang <elta.era@gmail.com>
---
 target-mips/dsp_helper.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/dsp_helper.c b/target-mips/dsp_helper.c
index 14daf91..536032b 100644
--- a/target-mips/dsp_helper.c
+++ b/target-mips/dsp_helper.c
@@ -2,7 +2,7 @@
  * MIPS ASE DSP Instruction emulation helpers for QEMU.
  *
  * Copyright (c) 2012  Jia Liu <proljc@gmail.com>
- *                     Dongxue Zhang <elat.era@gmail.com>
+ *                     Dongxue Zhang <elta.era@gmail.com>
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
  * License as published by the Free Software Foundation; either
-- 
1.7.10.4


[-- Attachment #3: 0002-Make-repl_ph-to-sign-extended-to-target_long.patch --]
[-- Type: application/octet-stream, Size: 1152 bytes --]

From 79381b64d4cb4592fe90f50690743bf038141a7c Mon Sep 17 00:00:00 2001
From: Dongxue Zhang <elta.era@gmail.com>
Date: Mon, 10 Dec 2012 16:30:26 +0800
Subject: [PATCH 2/3] Make-repl_ph-to-sign-extended-to-target_long
Content-Type: text/plain; charset="utf-8"

When two part of 16bit value make up a 32bit value, we need to make a type
conversion. Because the 32bit value was a unsigned value.

Signed-off-by: Dongxue Zhang <elta.era@gmail.com>
---
 target-mips/translate.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index 65e6725..d7de3cc 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13770,7 +13770,7 @@ static void gen_mipsdsp_bitinsn(CPUMIPSState *env, DisasContext *ctx,
             {
                 imm = (ctx->opcode >> 16) & 0x03FF;
                 tcg_gen_movi_tl(cpu_gpr[ret], \
-                                (target_long)((int32_t)imm << 16 | \
+                                (target_long)(int32_t)((int32_t)imm << 16 | \
                                 (uint32_t)(uint16_t)imm));
             }
             break;
-- 
1.7.10.4


[-- Attachment #4: 0003-Fix-gen_HILO-to-make-it-adapt-each-arch-which-use-ac.patch --]
[-- Type: application/octet-stream, Size: 6241 bytes --]

From 023778ded3abfb6b16b85f14528647e7a802ed3a Mon Sep 17 00:00:00 2001
From: Dongxue Zhang <elta.era@gmail.com>
Date: Mon, 10 Dec 2012 16:31:09 +0800
Subject: [PATCH 3/3] Fix-gen_HILO-to-make-it-adapt-each-arch-which-use-acc
Content-Type: text/plain; charset="utf-8"

The old gen_HILO was used by many arch, and they use acc[0] only. But now we
know the arch with dsp will have 4 acc reigster. So we need Fix gen_HILO.

When we add arch mipsdsp, we changed the gen_HILO, but now I found it may cause
a bug. Because we just get index of acc register from opcode, other arch to use
gen_HILO my have value at that bit, so, a bug cased.

Now I take acc to be a param of gen_HILO, other arch use gen_HILO with that
param set to 0, and dsp arch will set the value as which acc register it
selected.

TARGET_MIPS64 in gen_HILO changed into (TARGET_LONG_SIZE == 8), on 64bits arch,
acc register value sign-extended to 64bit value, save into register.

Signed-off-by: Dongxue Zhang <elta.era@gmail.com>
---
 target-mips/translate.c |   64 ++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 28 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index d7de3cc..8ab3725 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -2571,10 +2571,10 @@ static void gen_shift (CPUMIPSState *env, DisasContext *ctx, uint32_t opc,
 }
 
 /* Arithmetic on HI/LO registers */
-static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
+static void gen_HILO(DisasContext *ctx, uint32_t opc, int reg,
+                     unsigned int acc)
 {
     const char *opn = "hilo";
-    unsigned int acc;
 
     if (reg == 0 && (opc == OPC_MFHI || opc == OPC_MFLO)) {
         /* Treat as NOP. */
@@ -2582,19 +2582,9 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
         return;
     }
 
-    if (opc == OPC_MFHI || opc == OPC_MFLO) {
-        acc = ((ctx->opcode) >> 21) & 0x03;
-    } else {
-        acc = ((ctx->opcode) >> 11) & 0x03;
-    }
-
-    if (acc != 0) {
-        check_dsp(ctx);
-    }
-
     switch (opc) {
     case OPC_MFHI:
-#if defined(TARGET_MIPS64)
+#if (TARGET_LONG_SIZE == 8)
         if (acc != 0) {
             tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_HI[acc]);
         } else
@@ -2605,7 +2595,7 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
         opn = "mfhi";
         break;
     case OPC_MFLO:
-#if defined(TARGET_MIPS64)
+#if (TARGET_LONG_SIZE == 8)
         if (acc != 0) {
             tcg_gen_ext32s_tl(cpu_gpr[reg], cpu_LO[acc]);
         } else
@@ -2617,7 +2607,7 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
         break;
     case OPC_MTHI:
         if (reg != 0) {
-#if defined(TARGET_MIPS64)
+#if (TARGET_LONG_SIZE == 8)
             if (acc != 0) {
                 tcg_gen_ext32s_tl(cpu_HI[acc], cpu_gpr[reg]);
             } else
@@ -2632,7 +2622,7 @@ static void gen_HILO (DisasContext *ctx, uint32_t opc, int reg)
         break;
     case OPC_MTLO:
         if (reg != 0) {
-#if defined(TARGET_MIPS64)
+#if (TARGET_LONG_SIZE == 8)
             if (acc != 0) {
                 tcg_gen_ext32s_tl(cpu_LO[acc], cpu_gpr[reg]);
             } else
@@ -10134,7 +10124,7 @@ static int decode_mips16_opc (CPUMIPSState *env, DisasContext *ctx,
             gen_logic(env, ctx, OPC_NOR, rx, ry, 0);
             break;
         case RR_MFHI:
-            gen_HILO(ctx, OPC_MFHI, rx);
+            gen_HILO(ctx, OPC_MFHI, rx, 0);
             break;
         case RR_CNVT:
             switch (cnvt_op) {
@@ -10166,7 +10156,7 @@ static int decode_mips16_opc (CPUMIPSState *env, DisasContext *ctx,
             }
             break;
         case RR_MFLO:
-            gen_HILO(ctx, OPC_MFLO, rx);
+            gen_HILO(ctx, OPC_MFLO, rx, 0);
             break;
 #if defined (TARGET_MIPS64)
         case RR_DSRA:
@@ -10922,11 +10912,11 @@ static void gen_pool16c_insn (CPUMIPSState *env, DisasContext *ctx, int *is_bran
         break;
     case MFHI16 + 0:
     case MFHI16 + 1:
-        gen_HILO(ctx, OPC_MFHI, uMIPS_RS5(ctx->opcode));
+        gen_HILO(ctx, OPC_MFHI, uMIPS_RS5(ctx->opcode), 0);
         break;
     case MFLO16 + 0:
     case MFLO16 + 1:
-        gen_HILO(ctx, OPC_MFLO, uMIPS_RS5(ctx->opcode));
+        gen_HILO(ctx, OPC_MFLO, uMIPS_RS5(ctx->opcode), 0);
         break;
     case BREAK16:
         generate_exception(ctx, EXCP_BREAK);
@@ -11286,16 +11276,16 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs,
     case 0x35:
         switch (minor) {
         case MFHI32:
-            gen_HILO(ctx, OPC_MFHI, rs);
+            gen_HILO(ctx, OPC_MFHI, rs, 0);
             break;
         case MFLO32:
-            gen_HILO(ctx, OPC_MFLO, rs);
+            gen_HILO(ctx, OPC_MFLO, rs, 0);
             break;
         case MTHI32:
-            gen_HILO(ctx, OPC_MTHI, rs);
+            gen_HILO(ctx, OPC_MTHI, rs, 0);
             break;
         case MTLO32:
-            gen_HILO(ctx, OPC_MTLO, rs);
+            gen_HILO(ctx, OPC_MTLO, rs, 0);
             break;
         default:
             goto pool32axf_invalid;
@@ -14446,12 +14436,30 @@ static void decode_opc (CPUMIPSState *env, DisasContext *ctx, int *is_branch)
             break;
         case OPC_MFHI:          /* Move from HI/LO */
         case OPC_MFLO:
-            gen_HILO(ctx, op1, rd);
-            break;
+            {
+                unsigned int acc;
+                acc = ((ctx->opcode) >> 21) & 0x03;
+
+                if (acc != 0) {
+                    check_dsp(ctx);
+                }
+
+                gen_HILO(ctx, op1, rd, acc);
+                break;
+            }
         case OPC_MTHI:
         case OPC_MTLO:          /* Move to HI/LO */
-            gen_HILO(ctx, op1, rs);
-            break;
+            {
+                unsigned int acc;
+                acc = ((ctx->opcode) >> 11) & 0x03;
+
+                if (acc != 0) {
+                    check_dsp(ctx);
+                }
+
+                gen_HILO(ctx, op1, rs, acc);
+                break;
+            }
         case OPC_PMON:          /* Pmon entry point, also R4010 selsl */
 #ifdef MIPS_STRICT_STANDARD
             MIPS_INVAL("PMON / selsl");
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Qemu-devel] Some patch about mips, gen_HILO bug fix.
@ 2012-12-10 16:07 Jovanovic, Petar
  0 siblings, 0 replies; 4+ messages in thread
From: Jovanovic, Petar @ 2012-12-10 16:07 UTC (permalink / raw)
  To: qemu-devel@nongnu.org, elta.era@gmail.com
  Cc: aurelien@aurel32.net, chenwj@iis.sinica.edu.tw

> 0002-Make-repl_ph-to-sign-extended-to-target_long.patch
> 0003-Fix-gen_HILO-to-make-it-adapt-each-arch-which-use-ac.patch

Can you send examples/tests for the issues that you fix?
It makes easier to review if you provide a simple example of a failing test.


Petar


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-12-10 17:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-10  8:37 [Qemu-devel] Some patch about mips, gen_HILO bug fix Elta Era
2012-12-10  9:22 ` 陳韋任 (Wei-Ren Chen)
2012-12-10 17:41 ` Richard Henderson
  -- strict thread matches above, loose matches on Subject: below --
2012-12-10 16:07 Jovanovic, Petar

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).