Re: [PING^3 PATCH 1/1] sim/riscv: PR29595, Fix multiply instructions

[Tsukasa OI <research_trasio@irq.a4lg.com>]

On 2022/10/07 0:58, Palmer Dabbelt wrote:
> On Thu, 06 Oct 2022 03:33:00 PDT (-0700), research_trasio@irq.a4lg.com
> wrote:
>> Because of recent 'Zmmul' support, the simulator is now broken.  This is
>> caused by instruction classification changes:
>>
>> [Before]
>> -   INSN_CLASS_M     : multiply / divide
>> [After Zmmul]
>> -   INSN_CLASS_M     : divide
>> -   INSN_CLASS_ZMMUL : multiply
>>
>> The simulator checks the instruction class to execute an instruction:
>>
>> -   INSN_CLASS_I  : 'I'
>> -   INSN_CLASS_M  : 'M' (multiply / divide)
>> -   INSN_CLASS_A  : 'A'
>>
>> 'Zmmul' moved multiply instructions to INSN_CLASS_ZMMUL and that
>> instruction
>> class is not handled by the simulator.
>>
>> This commit handles INSN_CLASS_ZMMUL for all 'M' instructions and adds a
>> testcase to test all RV32M instructions run without any faults.
>> ---
>>  sim/riscv/sim-main.c        |  1 +
>>  sim/testsuite/riscv/m-ext.s | 18 ++++++++++++++++++
>>  2 files changed, 19 insertions(+)
>>  create mode 100644 sim/testsuite/riscv/m-ext.s
>>
>> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
>> index 30d2f1e1c9a..0156f791d4b 100644
>> --- a/sim/riscv/sim-main.c
>> +++ b/sim/riscv/sim-main.c
>> @@ -936,6 +936,7 @@ execute_one (SIM_CPU *cpu, unsigned_word iw, const
>> struct riscv_opcode *op)
>>      case INSN_CLASS_I:
>>        return execute_i (cpu, iw, op);
>>      case INSN_CLASS_M:
>> +    case INSN_CLASS_ZMMUL:
>>        return execute_m (cpu, iw, op);
>>      default:
>>        TRACE_INSN (cpu, "UNHANDLED EXTENSION: %d", op->insn_class);
>> diff --git a/sim/testsuite/riscv/m-ext.s b/sim/testsuite/riscv/m-ext.s
>> new file mode 100644
>> index 00000000000..b85397a32a0
>> --- /dev/null
>> +++ b/sim/testsuite/riscv/m-ext.s
>> @@ -0,0 +1,18 @@
>> +# check that the RV32M instructions run without any fault.
>> +# mach: riscv
>> +
>> +.include "testutils.inc"
>> +
>> +    start
>> +
>> +    .option    arch, +m
>> +    mul    x0, x1, x2
>> +    mulh    x0, x1, x2
>> +    mulhu    x0, x1, x2
>> +    mulhsu    x0, x1, x2
>> +    div    x0, x1, x2
>> +    divu    x0, x1, x2
>> +    rem    x0, x1, x2
>> +    remu    x0, x1, x2
>> +
>> +    pass
> 
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
> 
> though as we're talking about in this meeting, I'm not actually a gdbsim
> maintainer so I'm not sure I can formally approve it.
> 

Palmer,

I saw following files and thought you are one of the person who can
formally approve my patch.

Quoting sim/MAINTAINERS:
>                 SIM Maintainers
> 
> The simulator is part of the GDB project, so see the file
> gdb/MAINTAINERS for general information about maintaining these files....
> common          Frank Ch. Eigler <fche@redhat.com>
> *               (target, then global maintainers)

I know that Andrew and Palmer are RISC-V target maintainers of GDB so I
assumed you are responsible for this area.  But it seems... no one knows
exactly.  It seems very few people is interested in the simulator so...
well... for now, I will continue pinging until someone who thinks
responsible notices.

Thanks,
Tsukasa