[Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[陳韋任 (Wei-Ren Chen)]

Hi all,

  While reading microMIPS decoding, I found a possible wrong opcode
encoding. According to [1] page 337, the bits 13..12 for MULTU is
0x01 rather than 0x00. Please review, thanks.

[1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
    Application-Specific Extension to the microMIPS32 Architecture

Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
---
 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 f6fc0c2..01b48fa 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10385,7 +10385,7 @@ enum {
 
     /* bits 13..12 for 0x32 */
     MULT_ACC = 0x0,
-    MULTU_ACC = 0x0,
+    MULTU_ACC = 0x1,
 
     /* bits 15..12 for 0x2c */
     SEB = 0x2,
-- 
1.7.12.3

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Jia Liu]

Hi guys,

On Nov 14, 2012, at 10:49 AM, 陳韋任 (Wei-Ren Chen) <chenwj@iis.sinica.edu.tw> wrote:

> Hi all,
> 
>  While reading microMIPS decoding, I found a possible wrong opcode
> encoding. According to [1] page 337, the bits 13..12 for MULTU is
> 0x01 rather than 0x00. Please review, thanks.
> 
> [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
>    Application-Specific Extension to the microMIPS32 Architecture
> 
> Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> ---
> 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 f6fc0c2..01b48fa 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -10385,7 +10385,7 @@ enum {
> 
>     /* bits 13..12 for 0x32 */
>     MULT_ACC = 0x0,
> -    MULTU_ACC = 0x0,
> +    MULTU_ACC = 0x1,
> 
>     /* bits 15..12 for 0x2c */
>     SEB = 0x2,
> -- 
> 1.7.12.3


try this, it is a example using exist frame.

diff --git a/target-mips/translate.c b/target-mips/translate.c
index f6fc0c2..04b2aae 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10371,6 +10371,8 @@ enum {
     MFC0 = 0x03,
     MTC0 = 0x0b,
 
+    MICRODSP_ABSQ_S_QB_OP = 0x04;
+
     /* bits 13..12 for 0x01 */
     MFHI_ACC = 0x0,
     MFLO_ACC = 0x1,
@@ -10452,6 +10454,9 @@ enum {
     MFLO32 = 0x1,
     MTHI32 = 0x2,
     MTLO32 = 0x3,
+
+    /* bits 15..12 for MICRODSP_ABSQ_S_QB_OP */
+    MICRODSP_ABSQ_S_QB = 0x00;
 };
 
 /* POOL32B encoding of minor opcode field (bits 15..12) */
@@ -11274,6 +11279,14 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs,
             goto pool32axf_invalid;
         }
         break;
+    case MICRODSP_ABSQ_S_QB_OP:
+        switch (minor) {
+        case MICRODSP_ABSQ_S_QB: // ABSQ_S_QB
+            break;
+        default:
+            goto pool32axf_invalid;
+        }
+        break;
     default:
     pool32axf_invalid:
         MIPS_INVAL("pool32axf");

Regards,
Jia.

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Aurelien Jarno]

On Wed, Nov 14, 2012 at 10:49:55AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> Hi all,
> 
>   While reading microMIPS decoding, I found a possible wrong opcode
> encoding. According to [1] page 337, the bits 13..12 for MULTU is
> 0x01 rather than 0x00. Please review, thanks.
> 
> [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
>     Application-Specific Extension to the microMIPS32 Architecture

This is indeed correct, but I guess the page you meant is 166. Page 337
refers to the original MULTU instruction from the MIPS Architecture for
Programmers Volume II-B: The microMIPS32 Instruction Set.

On the other the microMIPS32 encoding for DSP instruction is not yet
supported.


> Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> ---
>  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 f6fc0c2..01b48fa 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -10385,7 +10385,7 @@ enum {
>  
>      /* bits 13..12 for 0x32 */
>      MULT_ACC = 0x0,
> -    MULTU_ACC = 0x0,
> +    MULTU_ACC = 0x1,
>  
>      /* bits 15..12 for 0x2c */
>      SEB = 0x2,
> -- 
> 1.7.12.3
> 

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

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Aurelien Jarno]

On Wed, Nov 14, 2012 at 01:38:44PM +0800, Jia Liu wrote:
> Hi guys,
> 
> On Nov 14, 2012, at 10:49 AM, 陳韋任 (Wei-Ren Chen) <chenwj@iis.sinica.edu.tw> wrote:
> 
> > Hi all,
> > 
> >  While reading microMIPS decoding, I found a possible wrong opcode
> > encoding. According to [1] page 337, the bits 13..12 for MULTU is
> > 0x01 rather than 0x00. Please review, thanks.
> > 
> > [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
> >    Application-Specific Extension to the microMIPS32 Architecture
> > 
> > Signed-off-by: Chen Wei-Ren <chenwj@iis.sinica.edu.tw>
> > ---
> > 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 f6fc0c2..01b48fa 100644
> > --- a/target-mips/translate.c
> > +++ b/target-mips/translate.c
> > @@ -10385,7 +10385,7 @@ enum {
> > 
> >     /* bits 13..12 for 0x32 */
> >     MULT_ACC = 0x0,
> > -    MULTU_ACC = 0x0,
> > +    MULTU_ACC = 0x1,
> > 
> >     /* bits 15..12 for 0x2c */
> >     SEB = 0x2,
> > -- 
> > 1.7.12.3
> 
> 
> try this, it is a example using exist frame.
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index f6fc0c2..04b2aae 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -10371,6 +10371,8 @@ enum {
>      MFC0 = 0x03,
>      MTC0 = 0x0b,
>  
> +    MICRODSP_ABSQ_S_QB_OP = 0x04;
> +
>      /* bits 13..12 for 0x01 */
>      MFHI_ACC = 0x0,
>      MFLO_ACC = 0x1,
> @@ -10452,6 +10454,9 @@ enum {
>      MFLO32 = 0x1,
>      MTHI32 = 0x2,
>      MTLO32 = 0x3,
> +
> +    /* bits 15..12 for MICRODSP_ABSQ_S_QB_OP */
> +    MICRODSP_ABSQ_S_QB = 0x00;
>  };
>  
>  /* POOL32B encoding of minor opcode field (bits 15..12) */
> @@ -11274,6 +11279,14 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs,
>              goto pool32axf_invalid;
>          }
>          break;
> +    case MICRODSP_ABSQ_S_QB_OP:
> +        switch (minor) {
> +        case MICRODSP_ABSQ_S_QB: // ABSQ_S_QB
> +            break;
> +        default:
> +            goto pool32axf_invalid;
> +        }
> +        break;
>      default:
>      pool32axf_invalid:
>          MIPS_INVAL("pool32axf");
> 

I don't really understand what this patch is supposed to do compared to
the original bug report and patch.

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

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[陳韋任 (Wei-Ren Chen)]

Hi Aurelien,

> I don't really understand what this patch is supposed to do compared to
> the original bug report and patch.

  He just replyed to the wrong mail, never mind.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[陳韋任 (Wei-Ren Chen)]

On Wed, Nov 14, 2012 at 05:45:21PM +0100, Aurelien Jarno wrote:
> On Wed, Nov 14, 2012 at 10:49:55AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> > Hi all,
> > 
> >   While reading microMIPS decoding, I found a possible wrong opcode
> > encoding. According to [1] page 337, the bits 13..12 for MULTU is
> > 0x01 rather than 0x00. Please review, thanks.
> > 
> > [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
> >     Application-Specific Extension to the microMIPS32 Architecture
> 
> This is indeed correct, but I guess the page you meant is 166. Page 337
> refers to the original MULTU instruction from the MIPS Architecture for
> Programmers Volume II-B: The microMIPS32 Instruction Set.
> 
> On the other the microMIPS32 encoding for DSP instruction is not yet
> supported.

  Maybe we can start cleanup it a little bit. ;) Is this OK to commit?

Regards,
chenwj 

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[陳韋任 (Wei-Ren Chen)]

Hi Aurelien,

  off the topic. Do you happen to know if GCC or whatever compiler
support microMIPS? I don't see any option used to produce microMIPS
code on GCC manual [1]. Thanks.

Regards,
chenwj

[1] http://gcc.gnu.org/onlinedocs/gcc/MIPS-Options.html

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Aurelien Jarno]

On Thu, Nov 15, 2012 at 07:57:04AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> On Wed, Nov 14, 2012 at 05:45:21PM +0100, Aurelien Jarno wrote:
> > On Wed, Nov 14, 2012 at 10:49:55AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> > > Hi all,
> > > 
> > >   While reading microMIPS decoding, I found a possible wrong opcode
> > > encoding. According to [1] page 337, the bits 13..12 for MULTU is
> > > 0x01 rather than 0x00. Please review, thanks.
> > > 
> > > [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
> > >     Application-Specific Extension to the microMIPS32 Architecture
> > 
> > This is indeed correct, but I guess the page you meant is 166. Page 337
> > refers to the original MULTU instruction from the MIPS Architecture for
> > Programmers Volume II-B: The microMIPS32 Instruction Set.
> > 
> > On the other the microMIPS32 encoding for DSP instruction is not yet
> > supported.
> 
>   Maybe we can start cleanup it a little bit. ;) Is this OK to commit?
> 

My point was that the patch what correct, but not the comment. I have
committed it after fixing the comment.

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

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[Aurelien Jarno]

On Thu, Nov 15, 2012 at 09:17:20AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> Hi Aurelien,
> 
>   off the topic. Do you happen to know if GCC or whatever compiler
> support microMIPS? I don't see any option used to produce microMIPS
> code on GCC manual [1]. Thanks.
> 

There are some patches floating around to add a -mmicromips option to
GCC, but they haven't been merged yet.

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

Re: [Qemu-trivial] [PATCH] Possible wrong microMIPS opcode encoding

[陳韋任 (Wei-Ren Chen)]

On Thu, Nov 15, 2012 at 02:53:38PM +0100, Aurelien Jarno wrote:
> On Thu, Nov 15, 2012 at 07:57:04AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> > On Wed, Nov 14, 2012 at 05:45:21PM +0100, Aurelien Jarno wrote:
> > > On Wed, Nov 14, 2012 at 10:49:55AM +0800, 陳韋任 (Wei-Ren Chen) wrote:
> > > > Hi all,
> > > > 
> > > >   While reading microMIPS decoding, I found a possible wrong opcode
> > > > encoding. According to [1] page 337, the bits 13..12 for MULTU is
> > > > 0x01 rather than 0x00. Please review, thanks.
> > > > 
> > > > [1] MIPS Architecture for Programmers VolumeIV-e: The MIPS DSP
> > > >     Application-Specific Extension to the microMIPS32 Architecture
> > > 
> > > This is indeed correct, but I guess the page you meant is 166. Page 337
> > > refers to the original MULTU instruction from the MIPS Architecture for
> > > Programmers Volume II-B: The microMIPS32 Instruction Set.
> > > 
> > > On the other the microMIPS32 encoding for DSP instruction is not yet
> > > supported.
> > 
> >   Maybe we can start cleanup it a little bit. ;) Is this OK to commit?
> > 
> 
> My point was that the patch what correct, but not the comment. I have
> committed it after fixing the comment.

  I saw your commit, thanks. btw, I found codesourcery MIPS GCC has a
option called "-mmicromips". Would you mind confirm this option does
work for me?

Regards,
chenwj
 
-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj