sim: riscv: Fix confusion with c.jal vs. c.addiw

Message ID AS8P193MB12855E2A907FE07B0D50372DE4092@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM
State New
Headers
Series sim: riscv: Fix confusion with c.jal vs. c.addiw |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 warning Patch is already merged

Commit Message

Bernd Edlinger April 15, 2024, 8:36 a.m. UTC
  There was apparently a confusion which cpu model uses
compressed JAL and which ADDIW.  Fixed that in execute_c,
case MATCH_C_JAL | MATCH_C_ADDIW.

Fixes 3224e32fb84f ("sim: riscv: Add support for compressed integer instructions")
---
 sim/riscv/sim-main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Andrew Burgess April 15, 2024, 8:56 a.m. UTC | #1
Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> There was apparently a confusion which cpu model uses
> compressed JAL and which ADDIW.  Fixed that in execute_c,
> case MATCH_C_JAL | MATCH_C_ADDIW.
>
> Fixes 3224e32fb84f ("sim: riscv: Add support for compressed integer
> instructions")

Thanks for splitting this off.  This looks good.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  sim/riscv/sim-main.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index adff99921c6..9fd2cc70657 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -1016,9 +1016,9 @@ execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>        TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
>        break;
>      case MATCH_C_JAL | MATCH_C_ADDIW:
> -      /* JAL and ADDIW have the same mask but are only available on RV64 or
> -	 RV32 respectively.  */
> -      if (RISCV_XLEN (cpu) == 64)
> +      /* JAL and ADDIW have the same mask but are only available on RV32 or
> +	 RV64 respectively.  */
> +      if (RISCV_XLEN (cpu) == 32)
>  	{
>  	  imm = EXTRACT_CJTYPE_IMM (iw);
>  	  TRACE_INSN (cpu, "c.jal %" PRIxTW,
> @@ -1027,7 +1027,7 @@ execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>  	  pc = riscv_cpu->pc + imm;
>  	  TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
>  	}
> -      else if (RISCV_XLEN (cpu) == 32)
> +      else if (RISCV_XLEN (cpu) == 64)
>  	{
>  	  imm = EXTRACT_CITYPE_IMM (iw);
>  	  TRACE_INSN (cpu, "c.addiw %s, %s, %#" PRIxTW ";  // %s += %#" PRIxTW,
> -- 
> 2.39.2
  
Bernd Edlinger April 15, 2024, 9:05 a.m. UTC | #2
On 4/15/24 10:56, Andrew Burgess wrote:
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> 
>> There was apparently a confusion which cpu model uses
>> compressed JAL and which ADDIW.  Fixed that in execute_c,
>> case MATCH_C_JAL | MATCH_C_ADDIW.
>>
>> Fixes 3224e32fb84f ("sim: riscv: Add support for compressed integer
>> instructions")
> 
> Thanks for splitting this off.  This looks good.
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
> 

Okay, no problem.  Pushed.

Thanks
Bernd.

> Thanks,
> Andrew
> 
> 
>> ---
>>  sim/riscv/sim-main.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
>> index adff99921c6..9fd2cc70657 100644
>> --- a/sim/riscv/sim-main.c
>> +++ b/sim/riscv/sim-main.c
>> @@ -1016,9 +1016,9 @@ execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>>        TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
>>        break;
>>      case MATCH_C_JAL | MATCH_C_ADDIW:
>> -      /* JAL and ADDIW have the same mask but are only available on RV64 or
>> -	 RV32 respectively.  */
>> -      if (RISCV_XLEN (cpu) == 64)
>> +      /* JAL and ADDIW have the same mask but are only available on RV32 or
>> +	 RV64 respectively.  */
>> +      if (RISCV_XLEN (cpu) == 32)
>>  	{
>>  	  imm = EXTRACT_CJTYPE_IMM (iw);
>>  	  TRACE_INSN (cpu, "c.jal %" PRIxTW,
>> @@ -1027,7 +1027,7 @@ execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>>  	  pc = riscv_cpu->pc + imm;
>>  	  TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
>>  	}
>> -      else if (RISCV_XLEN (cpu) == 32)
>> +      else if (RISCV_XLEN (cpu) == 64)
>>  	{
>>  	  imm = EXTRACT_CITYPE_IMM (iw);
>>  	  TRACE_INSN (cpu, "c.addiw %s, %s, %#" PRIxTW ";  // %s += %#" PRIxTW,
>> -- 
>> 2.39.2
>
  

Patch

diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index adff99921c6..9fd2cc70657 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -1016,9 +1016,9 @@  execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
       TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
       break;
     case MATCH_C_JAL | MATCH_C_ADDIW:
-      /* JAL and ADDIW have the same mask but are only available on RV64 or
-	 RV32 respectively.  */
-      if (RISCV_XLEN (cpu) == 64)
+      /* JAL and ADDIW have the same mask but are only available on RV32 or
+	 RV64 respectively.  */
+      if (RISCV_XLEN (cpu) == 32)
 	{
 	  imm = EXTRACT_CJTYPE_IMM (iw);
 	  TRACE_INSN (cpu, "c.jal %" PRIxTW,
@@ -1027,7 +1027,7 @@  execute_c (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
 	  pc = riscv_cpu->pc + imm;
 	  TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
 	}
-      else if (RISCV_XLEN (cpu) == 32)
+      else if (RISCV_XLEN (cpu) == 64)
 	{
 	  imm = EXTRACT_CITYPE_IMM (iw);
 	  TRACE_INSN (cpu, "c.addiw %s, %s, %#" PRIxTW ";  // %s += %#" PRIxTW,