GVSoC: LR.W returns its value in the wrong register

[jpf-h]

Hey @Aquaticfuller @DiyouS,

found when running some custom kernels.

The AMO shim forwards LR downstream by rewriting the opcode to a plain READ — but never fixed up the data pointers. For atomics, the ISS convention is get_data() = rs2 (operand in) and get_second_data() = rd (result out); a plain READ writes into get_data(). So the loaded value landed in the rs2 buffer and rd was left stale — lr.w returned garbage (whatever rd previously held). Any lr/sc-based lock or retry loop misbehaves: spurious success/failure depending on stale register content.

Fix. On the LR path, redirect set_data(get_second_data()) before forwarding — the same convention the AMO write-back and the generic memory.cpp handle_atomic already follow (which is why LR worked against plain memory but not through the shim).

Note: Found by AI, I have not verified the diagnosis and patch below completely and unfortunately don't have time to clean it up now. But I wanted to report before you run into this issue and have to debug it again :-)

From 917c5d10f999471b99e1f5dd20953b4f52ab6e8c Mon Sep 17 00:00:00 2001
From: Johannes Pfau <johannes.pfau@h-partners.com>
Date: Thu, 6 Aug 2026 10:22:15 +0200
Subject: [PATCH] insitu-cache: Fix handling of lr.w instruction

---
 models/cache/insitu/insitu_cache_amo_shim.cpp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/models/cache/insitu/insitu_cache_amo_shim.cpp b/models/cache/insitu/insitu_cache_amo_shim.cpp
index 1526a9de..da152bcc 100644
--- a/models/cache/insitu/insitu_cache_amo_shim.cpp
+++ b/models/cache/insitu/insitu_cache_amo_shim.cpp
@@ -147,6 +147,14 @@ vp::IoReqStatus InsituCacheAmo::req_handler(vp::Block *__this, vp::IoReq *req)
     if (op == vp::LR) {
         _this->res_.on_lr(core, addr);
         req->set_opcode(vp::READ);
+        // CRITICAL, same convention as the AMO write-back below (lsu.cpp:547-549, mirrored by
+        // memory.cpp's handle_atomic, which passes get_second_data() as its out_data for LR too):
+        // for an atomic, get_data() aliases rs2 and get_second_data() aliases rd. A plain READ
+        // writes into get_data() -- so forwarding LR untouched put the loaded value in rs2 and left
+        // rd stale.
+        if (req->get_second_data() != nullptr) {
+            req->set_data(req->get_second_data());
+        }
         return _this->output_.req_forward(req);
     }
 
-- 
2.50.1