diff mbox series

[2/4,sim/riscv] Fix JALR instruction simulation

Message ID CWXP265MB532192568CE81921F58C1F048CD6A@CWXP265MB5321.GBRP265.PROD.OUTLOOK.COM
State New
Headers
Series [1/4,sim/riscv] Add basic semi-hosting support |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Patch failed to apply

Commit Message

Jaydeep Patil Oct. 17, 2023, 5:53 a.m. UTC 
  Fixed 32bit JALR ra,ra,imm integer instruction, where RD was written before
using it to calculate target PC.
---
sim/riscv/sim-main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.25.1

Comments

Andrew Burgess Oct. 18, 2023, 4:58 p.m. UTC | #1 
Jaydeep Patil <Jaydeep.Patil@imgtec.com> writes:

> Fixed 32bit JALR ra,ra,imm integer instruction, where RD was written before
> using it to calculate target PC.
> ---
> sim/riscv/sim-main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 2b184aea554..3cf6e3fc4b0 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -598,8 +598,8 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
>        break;
>      case MATCH_JALR:
>        TRACE_INSN (cpu, "jalr %s, %s, %" PRIiTW ";", rd_name, rs1_name, i_imm);
> -      store_rd (cpu, rd, riscv_cpu->pc + 4);
>        pc = riscv_cpu->regs[rs1] + i_imm;
> +      store_rd (cpu, rd, riscv_cpu->pc + 4);
>        TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
>        break;
>

Thanks for fixing this.

I added a test for this change -- ideally change will come with an
associated test -- and pushed this patch.  What I pushed is below.

Thanks,
Andrew

---

commit 1c37b30945073f34bbb685d2ac47ab01e0c93d45
Author: Jaydeep Patil <Jaydeep.Patil@imgtec.com>
Date:   Wed Oct 18 17:37:59 2023 +0100

    sim/riscv: fix JALR instruction simulation
    
    Fix 32bit 'jalr rd,ra,imm' integer instruction, where RD was written
    before using it to calculate destination address.
    
    This commit also improves testutils.inc for riscv; make use of
    pushsection and popsection when adding things to .data, and setup the
    %gp global pointer register within the 'start' macro.
    
    Approved-By: Andrew Burgess <aburgess@redhat.com>

diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index 250791634a1..afdfcf50656 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -449,8 +449,8 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
       break;
     case MATCH_JALR:
       TRACE_INSN (cpu, "jalr %s, %s, %" PRIiTW ";", rd_name, rs1_name, i_imm);
-      store_rd (cpu, rd, riscv_cpu->pc + 4);
       pc = riscv_cpu->regs[rs1] + i_imm;
+      store_rd (cpu, rd, riscv_cpu->pc + 4);
       TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
       break;
 
diff --git a/sim/testsuite/riscv/jalr.s b/sim/testsuite/riscv/jalr.s
new file mode 100644
index 00000000000..daccf4fb5a0
--- /dev/null
+++ b/sim/testsuite/riscv/jalr.s
@@ -0,0 +1,22 @@
+# Basic jalr tests.
+# mach: riscv
+
+.include "testutils.inc"
+
+	start
+
+	# Load desination into register a0.
+	la	a0, good_dest
+
+	# Jump to the destination in a0.
+	jalr	a0, a0, 0
+
+	# If we write destination into a0 before reading it in order
+	# to jump, we might end up here.
+bad_dest:
+	fail
+
+	# We should end up here.
+good_dest:
+	pass
+	fail
diff --git a/sim/testsuite/riscv/testutils.inc b/sim/testsuite/riscv/testutils.inc
index b9680b9c22e..c5e09eb5ea3 100644
--- a/sim/testsuite/riscv/testutils.inc
+++ b/sim/testsuite/riscv/testutils.inc
@@ -21,8 +21,9 @@
 	# Trigger OS trap.
 	ecall;
 	exit 0;
-	.data
+	.pushsection .data
 	1: .asciz "pass\n"
+	.popsection
 	.endm
 
 # MACRO: fail
@@ -33,14 +34,15 @@
 	# Use stdout.
 	li a0, 1;
 	# Point to the string.
-	lla a1, 1f;
+	la a1, 1f;
 	# Number of bytes to write.
 	li a2, 5;
 	# Trigger OS trap.
 	ecall;
 	exit 0;
-	.data
+	.pushsection .data
 	1: .asciz "fail\n"
+	.popsection
 	.endm
 
 # MACRO: start
@@ -49,4 +51,8 @@
 	.text
 .global _start
 _start:
+	.option push
+	.option norelax
+	lla gp, __global_pointer$
+	.option pop
 	.endm
diff mbox series

Patch

diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index 2b184aea554..3cf6e3fc4b0 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -598,8 +598,8 @@  execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
       break;
     case MATCH_JALR:
       TRACE_INSN (cpu, "jalr %s, %s, %" PRIiTW ";", rd_name, rs1_name, i_imm);
-      store_rd (cpu, rd, riscv_cpu->pc + 4);
       pc = riscv_cpu->regs[rs1] + i_imm;
+      store_rd (cpu, rd, riscv_cpu->pc + 4);
       TRACE_BRANCH (cpu, "to %#" PRIxTW, pc);
       break;