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

2020-12-16 Thread Philippe Mathieu-Daudé
On 12/16/20 8:08 PM, Richard Henderson wrote:
> On 12/16/20 10:27 AM, Philippe Mathieu-Daudé wrote:
>> MIPS 64-bit ISA is introduced with MIPS3.
>> No need for another bit/definition to check for 64-bit.
>>
>> Suggested-by: Jiaxun Yang 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  target/mips/mips-defs.h | 2 +-
>>  hw/mips/boston.c| 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
>> index f4d76e562d1..ab621a750d5 100644
>> --- a/target/mips/mips-defs.h
>> +++ b/target/mips/mips-defs.h
>> @@ -19,7 +19,7 @@
>>   */
>>  #define ISA_MIPS1 0x0001ULL
>>  #define ISA_MIPS2 0x0002ULL
>> -#define ISA_MIPS3 0x0004ULL
>> +#define ISA_MIPS3 0x0004ULL /* 64-bit */
>>  #define ISA_MIPS4 0x0008ULL
>>  #define ISA_MIPS5 0x0010ULL
>>  #define ISA_MIPS320x0020ULL
>> 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);
> 
> Find this slightly confusing.
> 
> After all of the renaming, I would expect ISA_MIPS64R6 -> ISA_MIPS_R6 |
> ISA_MIPS_64, not ISA_MIPS_R6 | ISA_MIPS3.

Well all the ISA_* definitions now match:
https://images.anandtech.com/doci/8457/MIPS%20ISA%20Evolution.JPG

Except ISA_NANOMIPS32, which is listed in
MD01251-2B-nanoMIPS32PRA-06.09.pdf as an extension, similar to microMIPS:

  MIPS32, microMIPS32, and nanoMIPS32 Operating Modes

  Release 2 of the MIPS32 Architecture added support for 64-bit
  coprocessors (and, in particular, 64-bit floating-point units)
  with 32-bit CPUs. Thus, certain floating-point instructions
  that previously were enabled by 64-bit operations on a MIPS64
  processor now are enabled by new 64-bit floating-point operations.

  Release 3 introduced the microMIPS instruction set, allowing all
  microMIPS processors to implement a 64-bit floating-point unit.

  Release 6 introduces the nanoMIPS instruction set. The nanoMIPS
  instruction set provides access to the same instruction set extensions
  (example, COP1 floating-point instructions) that microMIPS had access
  to.

I'd rather keep one definitions per ISA. Eventually if you want
a definition to check if a CPU is 32/64-bit we can add an alias:

-- >8 --
diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index 376262fa250..2c3f4277cfe 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -65,6 +65,8 @@
 #define CPU_LOONGSON2E  (CPU_MIPS3 | INSN_LOONGSON2E)
 #define CPU_LOONGSON2F  (CPU_MIPS3 | INSN_LOONGSON2F | ASE_LMMI)

+#define CPU_MIPS64  (ISA_MIPS3)
+
 /* MIPS Technologies "Release 1" */
 #define CPU_MIPS32R1(CPU_MIPS2 | ISA_MIPS_R1)
 #define CPU_MIPS64R1(CPU_MIPS5 | CPU_MIPS32R1)
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index f44f681fab5..9f56099e42f 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_MIPS3);
+is_64b = cpu_type_supports_isa(machine->cpu_type, CPU_MIPS64);

 object_initialize_child(OBJECT(machine), "cps", >cps,
TYPE_MIPS_CPS);
 object_property_set_str(OBJECT(>cps), "cpu-type", machine->cpu_type,
---

But for the Boston case, it is simpler to add an inline function in
cpu.h:

-- >8 --
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1302,6 +1302,11 @@ static inline bool ase_mt_available(CPUMIPSState
*env)
 return env->CP0_Config3 & (1 << CP0C3_MT);
 }

+static inline bool cpu_type_is_64bit(const char *cpu_type)
+{
+return cpu_type_supports_isa(cpu_type, CPU_MIPS64);
+}
+
 void cpu_set_exception_base(int vp_index, target_ulong address);

 /* addr.c */
---

Note, I'd still use ISA_MIPS3 in this cpu_type_is_64bit().

Or I could add the ISA_MIPS_64 alias and call it a day...

> 
> 
> r~
> 



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

2020-12-16 Thread Richard Henderson
On 12/16/20 10:27 AM, Philippe Mathieu-Daudé wrote:
> MIPS 64-bit ISA is introduced with MIPS3.
> No need for another bit/definition to check for 64-bit.
> 
> Suggested-by: Jiaxun Yang 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/mips/mips-defs.h | 2 +-
>  hw/mips/boston.c| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
> index f4d76e562d1..ab621a750d5 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -19,7 +19,7 @@
>   */
>  #define ISA_MIPS1 0x0001ULL
>  #define ISA_MIPS2 0x0002ULL
> -#define ISA_MIPS3 0x0004ULL
> +#define ISA_MIPS3 0x0004ULL /* 64-bit */
>  #define ISA_MIPS4 0x0008ULL
>  #define ISA_MIPS5 0x0010ULL
>  #define ISA_MIPS320x0020ULL
> 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);

Find this slightly confusing.

After all of the renaming, I would expect ISA_MIPS64R6 -> ISA_MIPS_R6 |
ISA_MIPS_64, not ISA_MIPS_R6 | ISA_MIPS3.


r~



[PATCH v2 02/12] target/mips/mips-defs: Use ISA_MIPS3 for ISA_MIPS64

2020-12-16 Thread Philippe Mathieu-Daudé
MIPS 64-bit ISA is introduced with MIPS3.
No need for another bit/definition to check for 64-bit.

Suggested-by: Jiaxun Yang 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/mips-defs.h | 2 +-
 hw/mips/boston.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
index f4d76e562d1..ab621a750d5 100644
--- a/target/mips/mips-defs.h
+++ b/target/mips/mips-defs.h
@@ -19,7 +19,7 @@
  */
 #define ISA_MIPS1 0x0001ULL
 #define ISA_MIPS2 0x0002ULL
-#define ISA_MIPS3 0x0004ULL
+#define ISA_MIPS3 0x0004ULL /* 64-bit */
 #define ISA_MIPS4 0x0008ULL
 #define ISA_MIPS5 0x0010ULL
 #define ISA_MIPS320x0020ULL
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", >cps, TYPE_MIPS_CPS);
 object_property_set_str(OBJECT(>cps), "cpu-type", machine->cpu_type,
-- 
2.26.2