[Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes

Aleksandar Markovic posted 5 patches 7 years ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Aleksandar Markovic 7 years ago
From: Aleksandar Markovic <amarkovic@wavecomp.com>

Misc changes in comments and strings for R5900.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 target/mips/translate.c          | 14 +++++++-------
 target/mips/translate_init.inc.c | 12 ------------
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 155331f..259ad2b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -2096,8 +2096,8 @@ enum {
  * MTSAH   rs, immediate     Move Halfword Count to Shift Amount Register
  * PROT3W  rd, rt            Parallel Rotate 3 Words
  *
- *     The TX79-specific Multimedia Instruction encodings
- *     ==================================================
+ *     Multimedia Instructions (MMI) encodings
+ *     =======================================
  *
  * MMI Instruction encoding table keys:
  *
@@ -24714,7 +24714,7 @@ static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI0 */
         break;
     default:
-        MIPS_INVAL("TX79 MMI class MMI0");
+        MIPS_INVAL("MMI class MMI0");
         generate_exception_end(ctx, EXCP_RI);
         break;
     }
@@ -24746,7 +24746,7 @@ static void decode_mmi1(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI1 */
         break;
     default:
-        MIPS_INVAL("TX79 MMI class MMI1");
+        MIPS_INVAL("MMI class MMI1");
         generate_exception_end(ctx, EXCP_RI);
         break;
     }
@@ -24782,7 +24782,7 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI2 */
         break;
     default:
-        MIPS_INVAL("TX79 MMI class MMI2");
+        MIPS_INVAL("MMI class MMI2");
         generate_exception_end(ctx, EXCP_RI);
         break;
     }
@@ -24809,7 +24809,7 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI3 */
         break;
     default:
-        MIPS_INVAL("TX79 MMI class MMI3");
+        MIPS_INVAL("MMI class MMI3");
         generate_exception_end(ctx, EXCP_RI);
         break;
     }
@@ -24867,7 +24867,7 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
         generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_CLASS_MMI */
         break;
     default:
-        MIPS_INVAL("TX79 MMI class");
+        MIPS_INVAL("MMI class");
         generate_exception_end(ctx, EXCP_RI);
         break;
     }
diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
index 85da4a2..cab2003 100644
--- a/target/mips/translate_init.inc.c
+++ b/target/mips/translate_init.inc.c
@@ -411,18 +411,6 @@ const mips_def_t mips_defs[] =
         .mmu_type = MMU_TYPE_R4000,
     },
     {
-        /*
-         * The Toshiba TX System RISC TX79 Core Architecture manual
-         *
-         * https://wiki.qemu.org/File:C790.pdf
-         *
-         * describes the C790 processor that is a follow-up to the R5900.
-         * There are a few notable differences in that the R5900 FPU
-         *
-         * - is not IEEE 754-1985 compliant,
-         * - does not implement double format, and
-         * - its machine code is nonstandard.
-         */
         .name = "R5900",
         .CP0_PRid = 0x00002E00,
         /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Philippe Mathieu-Daudé 7 years ago
On 30/10/18 12:36, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> Misc changes in comments and strings for R5900.
> 
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>   target/mips/translate.c          | 14 +++++++-------
>   target/mips/translate_init.inc.c | 12 ------------
>   2 files changed, 7 insertions(+), 19 deletions(-)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 155331f..259ad2b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -2096,8 +2096,8 @@ enum {
>    * MTSAH   rs, immediate     Move Halfword Count to Shift Amount Register
>    * PROT3W  rd, rt            Parallel Rotate 3 Words
>    *
> - *     The TX79-specific Multimedia Instruction encodings
> - *     ==================================================
> + *     Multimedia Instructions (MMI) encodings

Oh now I see this. I this single change should be squashed into patch #2 
of this series.

Also, maybe use "MultiMedia ..."

> + *     =======================================
>    *
>    * MMI Instruction encoding table keys:
>    *
> @@ -24714,7 +24714,7 @@ static void decode_mmi0(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI0 */
>           break;
>       default:
> -        MIPS_INVAL("TX79 MMI class MMI0");
> +        MIPS_INVAL("MMI class MMI0");
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       }
> @@ -24746,7 +24746,7 @@ static void decode_mmi1(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI1 */
>           break;
>       default:
> -        MIPS_INVAL("TX79 MMI class MMI1");
> +        MIPS_INVAL("MMI class MMI1");
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       }
> @@ -24782,7 +24782,7 @@ static void decode_mmi2(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI2 */
>           break;
>       default:
> -        MIPS_INVAL("TX79 MMI class MMI2");
> +        MIPS_INVAL("MMI class MMI2");
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       }
> @@ -24809,7 +24809,7 @@ static void decode_mmi3(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI); /* TODO: MMI_OPC_CLASS_MMI3 */
>           break;
>       default:
> -        MIPS_INVAL("TX79 MMI class MMI3");
> +        MIPS_INVAL("MMI class MMI3");
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       }
> @@ -24867,7 +24867,7 @@ static void decode_mmi(CPUMIPSState *env, DisasContext *ctx)
>           generate_exception_end(ctx, EXCP_RI);    /* TODO: MMI_CLASS_MMI */
>           break;
>       default:
> -        MIPS_INVAL("TX79 MMI class");
> +        MIPS_INVAL("MMI class");
>           generate_exception_end(ctx, EXCP_RI);
>           break;
>       }
> diff --git a/target/mips/translate_init.inc.c b/target/mips/translate_init.inc.c
> index 85da4a2..cab2003 100644
> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -411,18 +411,6 @@ const mips_def_t mips_defs[] =
>           .mmu_type = MMU_TYPE_R4000,
>       },
>       {
> -        /*
> -         * The Toshiba TX System RISC TX79 Core Architecture manual
> -         *
> -         * https://wiki.qemu.org/File:C790.pdf
> -         *
> -         * describes the C790 processor that is a follow-up to the R5900.
> -         * There are a few notable differences in that the R5900 FPU
> -         *
> -         * - is not IEEE 754-1985 compliant,
> -         * - does not implement double format, and
> -         * - its machine code is nonstandard.
> -         */

Why remove this documentation? This entry is specific to the R5900.

>           .name = "R5900",
>           .CP0_PRid = 0x00002E00,
>           /* No L2 cache, icache size 32k, dcache size 32k, uncached coherency. */
> 

Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Philippe Mathieu-Daudé 7 years ago
On 30/10/18 13:47, Philippe Mathieu-Daudé wrote:
> On 30/10/18 12:36, Aleksandar Markovic wrote:
>> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>>
>> Misc changes in comments and strings for R5900.
>>
>> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>> ---
>>   target/mips/translate.c          | 14 +++++++-------
>>   target/mips/translate_init.inc.c | 12 ------------
>>   2 files changed, 7 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 155331f..259ad2b 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -2096,8 +2096,8 @@ enum {
>>    * MTSAH   rs, immediate     Move Halfword Count to Shift Amount 
>> Register
>>    * PROT3W  rd, rt            Parallel Rotate 3 Words
>>    *
>> - *     The TX79-specific Multimedia Instruction encodings
>> - *     ==================================================
>> + *     Multimedia Instructions (MMI) encodings
> 
> Oh now I see this. I this single change should be squashed into patch #2 

I forgot the verb ;) I <think> this change ...

> of this series.
> 
> Also, maybe use "MultiMedia ..."
> 

Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Philippe Mathieu-Daudé 7 years ago
On 30/10/18 13:47, Philippe Mathieu-Daudé wrote:
> On 30/10/18 12:36, Aleksandar Markovic wrote:
>> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>>
>> Misc changes in comments and strings for R5900.
>>
>> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>> ---
>>   target/mips/translate.c          | 14 +++++++-------
>>   target/mips/translate_init.inc.c | 12 ------------
>>   2 files changed, 7 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 155331f..259ad2b 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -2096,8 +2096,8 @@ enum {
>>    * MTSAH   rs, immediate     Move Halfword Count to Shift Amount 
>> Register
>>    * PROT3W  rd, rt            Parallel Rotate 3 Words
>>    *
>> - *     The TX79-specific Multimedia Instruction encodings
>> - *     ==================================================
>> + *     Multimedia Instructions (MMI) encodings
> 
> Oh now I see this. I this single change should be squashed into patch #2 
> of this series.
> 
> Also, maybe use "MultiMedia ..."
> 
>> + *     =======================================
>>    *
>>    * MMI Instruction encoding table keys:
>>    *

The part from here ...

>> @@ -24714,7 +24714,7 @@ static void decode_mmi0(CPUMIPSState *env, 
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO: 
>> MMI_OPC_CLASS_MMI0 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI0");
>> +        MIPS_INVAL("MMI class MMI0");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24746,7 +24746,7 @@ static void decode_mmi1(CPUMIPSState *env, 
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO: 
>> MMI_OPC_CLASS_MMI1 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI1");
>> +        MIPS_INVAL("MMI class MMI1");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24782,7 +24782,7 @@ static void decode_mmi2(CPUMIPSState *env, 
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO: 
>> MMI_OPC_CLASS_MMI2 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI2");
>> +        MIPS_INVAL("MMI class MMI2");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24809,7 +24809,7 @@ static void decode_mmi3(CPUMIPSState *env, 
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO: 
>> MMI_OPC_CLASS_MMI3 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI3");
>> +        MIPS_INVAL("MMI class MMI3");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24867,7 +24867,7 @@ static void decode_mmi(CPUMIPSState *env, 
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI);    /* TODO: 
>> MMI_CLASS_MMI */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class");
>> +        MIPS_INVAL("MMI class");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }

... to here can also be squashed in patch #3.

>> diff --git a/target/mips/translate_init.inc.c 
>> b/target/mips/translate_init.inc.c
>> index 85da4a2..cab2003 100644
>> --- a/target/mips/translate_init.inc.c
>> +++ b/target/mips/translate_init.inc.c
>> @@ -411,18 +411,6 @@ const mips_def_t mips_defs[] =
>>           .mmu_type = MMU_TYPE_R4000,
>>       },
>>       {
>> -        /*
>> -         * The Toshiba TX System RISC TX79 Core Architecture manual
>> -         *
>> -         * https://wiki.qemu.org/File:C790.pdf
>> -         *
>> -         * describes the C790 processor that is a follow-up to the 
>> R5900.
>> -         * There are a few notable differences in that the R5900 FPU
>> -         *
>> -         * - is not IEEE 754-1985 compliant,
>> -         * - does not implement double format, and
>> -         * - its machine code is nonstandard.
>> -         */
> 
> Why remove this documentation? This entry is specific to the R5900.
> 
>>           .name = "R5900",
>>           .CP0_PRid = 0x00002E00,
>>           /* No L2 cache, icache size 32k, dcache size 32k, uncached 
>> coherency. */
>>

Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Aleksandar Markovic 7 years ago
>> --- a/target/mips/translate_init.inc.c
>> +++ b/target/mips/translate_init.inc.c
>> @@ -411,18 +411,6 @@ const mips_def_t mips_defs[] =
>>           .mmu_type = MMU_TYPE_R4000,
>>       },
>>       {
>> -        /*
>> -         * The Toshiba TX System RISC TX79 Core Architecture manual
>> -         *
>> -         * https://wiki.qemu.org/File:C790.pdf
>> -         *
>> -         * describes the C790 processor that is a follow-up to the
>> R5900.
>> -         * There are a few notable differences in that the R5900 FPU
>> -         *
>> -         * - is not IEEE 754-1985 compliant,
>> -         * - does not implement double format, and
>> -         * - its machine code is nonstandard.
>> -         */
>
> Why remove this documentation? This entry is specific to the R5900.
>

The doc content is virtually duplicated in translate.c

The FPU content is virtually duplicated twenty lines below.

This file is not a place suitable for extensive commenting.

Aleksandar
Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes
Posted by Aleksandar Markovic 7 years ago
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Subject: Re: [Qemu-devel] [PATCH v2 4/5] target/mips: Misc R5900-related cosmetic changes

Sure, I will move these parts from patch to patch.

> On 30/10/18 13:47, Philippe Mathieu-Daudé wrote:
> On 30/10/18 12:36, Aleksandar Markovic wrote:
>> From: Aleksandar Markovic <amarkovic@wavecomp.com>
>>
>> Misc changes in comments and strings for R5900.
>>
>> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
>> ---
>>   target/mips/translate.c          | 14 +++++++-------
>>   target/mips/translate_init.inc.c | 12 ------------
>>   2 files changed, 7 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 155331f..259ad2b 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -2096,8 +2096,8 @@ enum {
>>    * MTSAH   rs, immediate     Move Halfword Count to Shift Amount
>> Register
>>    * PROT3W  rd, rt            Parallel Rotate 3 Words
>>    *
>> - *     The TX79-specific Multimedia Instruction encodings
>> - *     ==================================================
>> + *     Multimedia Instructions (MMI) encodings
>
> Oh now I see this. I this single change should be squashed into patch #2
> of this series.
>
> Also, maybe use "MultiMedia ..."
>

Sure.

>> + *     =======================================
>>    *
>>    * MMI Instruction encoding table keys:
>>    *

> The part from here ...

>> @@ -24714,7 +24714,7 @@ static void decode_mmi0(CPUMIPSState *env,
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO:
>> MMI_OPC_CLASS_MMI0 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI0");
>> +        MIPS_INVAL("MMI class MMI0");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24746,7 +24746,7 @@ static void decode_mmi1(CPUMIPSState *env,
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO:
>> MMI_OPC_CLASS_MMI1 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI1");
>> +        MIPS_INVAL("MMI class MMI1");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24782,7 +24782,7 @@ static void decode_mmi2(CPUMIPSState *env,
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO:
>> MMI_OPC_CLASS_MMI2 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI2");
>> +        MIPS_INVAL("MMI class MMI2");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24809,7 +24809,7 @@ static void decode_mmi3(CPUMIPSState *env,
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI); /* TODO:
>> MMI_OPC_CLASS_MMI3 */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class MMI3");
>> +        MIPS_INVAL("MMI class MMI3");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }
>> @@ -24867,7 +24867,7 @@ static void decode_mmi(CPUMIPSState *env,
>> DisasContext *ctx)
>>           generate_exception_end(ctx, EXCP_RI);    /* TODO:
>> MMI_CLASS_MMI */
>>           break;
>>       default:
>> -        MIPS_INVAL("TX79 MMI class");
>> +        MIPS_INVAL("MMI class");
>>           generate_exception_end(ctx, EXCP_RI);
>>           break;
>>       }

> ... to here can also be squashed in patch #3.

Sure.