Re: [PATCH 2/2] sim: riscv: Simplify the signed div by -1 code

[Andrew Burgess <aburgess@redhat.com>]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> This uses the idea from the previous patch to
> simplify the code for non-overflowing signed
> divisions by -1.  This is no bug-fix but it
> simplifies the code and avoids some unnecessary
> branches.
> ---
>  sim/riscv/sim-main.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 515ff835223..e4b15b533ba 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -700,18 +700,16 @@ execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>    const char *rd_name = riscv_gpr_names_abi[rd];
>    const char *rs1_name = riscv_gpr_names_abi[rs1];
>    const char *rs2_name = riscv_gpr_names_abi[rs2];
> -  unsigned_word tmp, dividend_max;
> +  unsigned_word tmp;
>    sim_cia pc = riscv_cpu->pc + 4;
>  
> -  dividend_max = -((unsigned_word) 1 << (WITH_TARGET_WORD_BITSIZE - 1));

I really don't understand what this was trying to calculate.  It's not
the max value of an unsigned_word.  Nor is it the bit representation of
the maximum signed_word value... but you're removing it, so I guess I
can not worry about that :)

> -
>    switch (op->match)
>      {
>      case MATCH_DIV:
>        TRACE_INSN (cpu, "div %s, %s, %s;  // %s = %s / %s",
>  		  rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name);
> -      if (riscv_cpu->regs[rs1] == dividend_max && riscv_cpu->regs[rs2] == -1)
> -	tmp = dividend_max;
> +      if (riscv_cpu->regs[rs2] == -1)
> +	tmp = -riscv_cpu->regs[rs1];

As with the previous commit, can't we drop this whole if check and block?

I think we should add some tests for this too.  The test I proposed in
the previous commit can be modified to give us div.s and rem.s tests.

Thanks,
Andrew

>        else if (riscv_cpu->regs[rs2])
>  	tmp = (signed_word) riscv_cpu->regs[rs1] /
>  	  (signed_word) riscv_cpu->regs[rs2];
> @@ -793,7 +791,7 @@ execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>      case MATCH_REM:
>        TRACE_INSN (cpu, "rem %s, %s, %s;  // %s = %s %% %s",
>  		  rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name);
> -      if (riscv_cpu->regs[rs1] == dividend_max && riscv_cpu->regs[rs2] == -1)
> +      if (riscv_cpu->regs[rs2] == -1)
>  	tmp = 0;
>        else if (riscv_cpu->regs[rs2])
>  	tmp = (signed_word) riscv_cpu->regs[rs1]
> -- 
> 2.39.2