Re: [PATCH 02/11] target/mips/mips-defs: Use ISA_MIPS3 for ISA_MIPS64

[Philippe Mathieu-Daudé]

On 12/16/20 4:06 PM, Jiaxun Yang wrote:
> 在 2020/12/16 21:43, Philippe Mathieu-Daudé 写道:
>> MIPS 64-bit ISA is introduced with MIPS3.
>> No need for another bit/definition to check for 64-bit.
>>
>> Simplify CPU_MIPS64 definition as CPU_MIPS5 contains
>> CPU_MIPS4 which contains CPU_MIPS3 which contains ISA_MIPS3.
> 
> Oops, I shall amend my reviewed by tag, just found a issue.
> 
>>
>> Suggested-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   target/mips/mips-defs.h |  5 ++---
>>   hw/mips/boston.c        |  2 +-
>>   target/mips/translate.c | 10 +++++-----
>>   3 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
>> index f4d76e562d1..2756e72a9d6 100644
>> --- a/target/mips/mips-defs.h
>> +++ b/target/mips/mips-defs.h
>> @@ -19,12 +19,11 @@
>>    */
>>   #define ISA_MIPS1         0x0000000000000001ULL
>>   #define ISA_MIPS2         0x0000000000000002ULL
>> -#define ISA_MIPS3         0x0000000000000004ULL
>> +#define ISA_MIPS3         0x0000000000000004ULL /* 64-bit */
>>   #define ISA_MIPS4         0x0000000000000008ULL
>>   #define ISA_MIPS5         0x0000000000000010ULL
>>   #define ISA_MIPS32        0x0000000000000020ULL
>>   #define ISA_MIPS32R2      0x0000000000000040ULL
>> -#define ISA_MIPS64        0x0000000000000080ULL
>>   #define ISA_MIPS64R2      0x0000000000000100ULL
>>   #define ISA_MIPS32R3      0x0000000000000200ULL
>>   #define ISA_MIPS64R3      0x0000000000000400ULL
>> @@ -78,7 +77,7 @@
>>     /* MIPS Technologies "Release 1" */
>>   #define CPU_MIPS32      (CPU_MIPS2 | ISA_MIPS32)
>> -#define CPU_MIPS64      (CPU_MIPS5 | CPU_MIPS32 | ISA_MIPS64)
>> +#define CPU_MIPS64      (CPU_MIPS5 | CPU_MIPS32)
>>     /* MIPS Technologies "Release 2" */
>>   #define CPU_MIPS32R2    (CPU_MIPS32 | ISA_MIPS32R2)
>> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>> index c3b94c68e1b..f44f681fab5 100644
>> --- a/hw/mips/boston.c
>> +++ b/hw/mips/boston.c
>> @@ -463,7 +463,7 @@ static void boston_mach_init(MachineState *machine)
>>           exit(1);
>>       }
>>   -    is_64b = cpu_type_supports_isa(machine->cpu_type, ISA_MIPS64);
>> +    is_64b = cpu_type_supports_isa(machine->cpu_type, ISA_MIPS3);
>>         object_initialize_child(OBJECT(machine), "cps", &s->cps,
>> TYPE_MIPS_CPS);
>>       object_property_set_str(OBJECT(&s->cps), "cpu-type",
>> machine->cpu_type,
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 15265485f76..8c0ecfa17e1 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -8538,7 +8538,7 @@ static void gen_dmfc0(DisasContext *ctx, TCGv
>> arg, int reg, int sel)
>>       const char *register_name = "invalid";
>>         if (sel != 0) {
>> -        check_insn(ctx, ISA_MIPS64);
>> +        check_insn(ctx, ISA_MIPS3);
>>       }
> 
> I think these ISA_MIPS64 should mean ISA_MIPS_R1?

Good catch, thanks.