Repository: amaranth-lang/amaranth (python)

Issue:
pysim LHS compiler miscompiles `Slice` of  `SwitchValue`
Add to `test_sim.py` `SimulatorUnitTestCase`:

```py
    def test_array_lhs_heterogenous_slice(self):
        l = Signal(1, init=1)
        m = Signal(3, init=4)
        n = Signal(5, init=7)
        array = Array([l, m, n])
        stmt = lambda y, a, b: [array[a].as_value()[2:].eq(b), y.eq(Cat(*array))]
        self.assertStatement(stmt, [C(0), C(0b000)], C(0b001111001, 9))
        self.assertStatement(stmt, [C(1), C(0b010)], C(0b001110001, 9))
        self.assertStatement(stmt, [C(2), C(0b100)], C(0b100111001, 9))
```

When the slice is assigned, the sliced value is first read using `next` mode, including the test value (array index), which is not an LHS, and thus does not have a `next_*` variable:

```
  File "/tmp/amaranth_pysim_sw1hg_w2", line 5, in run
    test_0 = 0x1 & next_4
                   ^^^^^^
NameError: name 'next_4' is not defined
```

```py
def run():
    next_0 = 1
    next_1 = 4
    next_2 = 0
    test_0 = 0x1 & next_4
    rhs_switch_1 = 0
    match test_0:
        case 0b00:
            rhs_switch_1 = (0x1 & next_0)
        case 0b01:
            rhs_switch_1 = (0x7 & next_1)
    test_2 = 0x1 & slots[4].curr
    match test_2:
        case 0b00:
            next_0 = 0x1 & (rhs_switch_1 & -0x5 | ((0x1 & (0x1 & slots[3].curr)) << 2))
        case 0b01:
            next_1 = 0x7 & (rhs_switch_1 & -0x5 | ((0x1 & (0x1 & slots[3].curr)) << 2))
    next_2 = 0x1ff & (0x1ff & (((0x1 & slots[0].curr) << 0) | ((0x7 & slots[1].curr) << 1) | ((0x1f & slots[5].curr) << 4)))
    slots[0].set(next_0)
    slots[1].set(next_1)
    slots[2].set(next_2)
```

Pull request description:
sim: fix LRHS evaluation.
Fixes #1269.

Patch:
diff --git a/amaranth/sim/_pyrtl.py b/amaranth/sim/_pyrtl.py
index f6c4943..5505cfb 100644
--- a/amaranth/sim/_pyrtl.py
+++ b/amaranth/sim/_pyrtl.py
@@ -153,12 +153,16 @@ class _ValueCompiler(ValueVisitor, _Compiler):
 
 
 class _RHSValueCompiler(_ValueCompiler):
-    def __init__(self, state, emitter, *, mode, inputs=None):
+    def __init__(self, state, emitter, *, mode, inputs=None, rrhs=None):
         super().__init__(state, emitter)
         assert mode in ("curr", "next")
         self.mode = mode
         # If not None, `inputs` gets populated with RHS signals.
         self.inputs = inputs
+        # When this compiler is used to grab the "next" value from within _LHSValueCompiler,
+        # we still need to use "curr" mode for reading part offsets etc. Allow setting a separate
+        # _RhsValueCompiler for these contexts.
+        self.rrhs = rrhs or self
 
     def sign(self, value):
         value_mask = (1 << len(value)) - 1
@@ -251,7 +255,7 @@ class _RHSValueCompiler(_ValueCompiler):
 
     def on_Part(self, value):
         offset_mask = (1 << len(value.offset)) - 1
-        offset = f"({value.stride} * ({offset_mask:#x} & {self(value.offset)}))"
+        offset = f"({value.stride} * ({offset_mask:#x} & {self.rrhs(value.offset)}))"
         return f"({(1 << value.width) - 1} & " \
                f"{self(value.value)} >> {offset})"
 
@@ -267,7 +271,7 @@ class _RHSValueCompiler(_ValueCompiler):
         return f"0"
 
     def on_SwitchValue(self, value):
-        gen_test = self.emitter.def_var("test", f"{(1 << len(value.test)) - 1:#x} & {self(value.test)}")
+        gen_test = self.emitter.def_var("test", f"{(1 << len(value.test)) - 1:#x} & {self.rrhs(value.test)}")
         gen_value = self.emitter.def_var("rhs_switch", "0")
         def case_handler(patterns, elem):
             self.emitter.append(f"{gen_value} = {self.sign(elem)}")
@@ -290,7 +294,7 @@ class _LHSValueCompiler(_ValueCompiler):
         self.rrhs = rhs
         # `lrhs` is used to translate the read part of a read-modify-write cycle during partial
         # update of an lvalue.
-        self.lrhs = _RHSValueCompiler(state, emitter, mode="next", inputs=None)
+        self.lrhs = _RHSValueCompiler(state, emitter, mode="next", inputs=None, rrhs=rhs)
         # If not None, `outputs` gets populated with signals on LHS.
         self.outputs = outputs
 


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index fa4f881..6b9affd 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -366,6 +366,16 @@ class SimulatorUnitTestCase(FHDLTestCase):
         self.assertStatement(stmt, [C(1), C(0b010)], C(0b001110101, 9))
         self.assertStatement(stmt, [C(2), C(0b100)], C(0b001001001, 9))
 
+    def test_array_lhs_heterogenous_slice(self):
+        l = Signal(1, init=1)
+        m = Signal(3, init=4)
+        n = Signal(5, init=7)
+        array = Array([l, m, n])
+        stmt = lambda y, a, b: [array[a].as_value()[2:].eq(b), y.eq(Cat(*array))]
+        self.assertStatement(stmt, [C(0), C(0b000)], C(0b001111001, 9))
+        self.assertStatement(stmt, [C(1), C(0b010)], C(0b001110001, 9))
+        self.assertStatement(stmt, [C(2), C(0b100)], C(0b100111001, 9))
+
     def test_array_lhs_oob(self):
         l = Signal(3)
         m = Signal(3)