Repository: amaranth-lang/amaranth
Pull request #827 (2023-06-24 03:33:33)

sim._pyrtl: mask bitwise binary operands.
Addresses the bug surfaced in https://github.com/amaranth-lang/amaranth/pull/826#issuecomment-1603634393.

I read through the remaining value compiler branches (and poked at a few things to see if I could break them), but I believe it's all OK from here.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/827?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) Report
> Merging [#827](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/827?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (70513e9) into [main](https://app.codecov.io/gh/amaranth-lang/amaranth/commit/4ec9cbbffe80985f791ce22f8c328520ee7bd0d6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) (4ec9cbb) will **decrease** coverage by `0.09%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #827      +/-   ##
==========================================
- Coverage   82.08%   82.00%   -0.09%     
==========================================
  Files          50       50              
  Lines        6839     6839              
  Branches     1667     1443     -224     
==========================================
- Hits         5614     5608       -6     
- Misses       1032     1035       +3     
- Partials      193      196       +3     
```


| [Impacted Files](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/827?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang) | Coverage Δ | |
|---|---|---|
| [amaranth/sim/\_pyrtl.py](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/827?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang#diff-YW1hcmFudGgvc2ltL19weXJ0bC5weQ==) | `95.42% <100.00%> (ø)` | |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/amaranth-lang/amaranth/pull/827/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=amaranth-lang)


Patch:
diff --git a/amaranth/sim/_pyrtl.py b/amaranth/sim/_pyrtl.py
index 1545686..d9787ff 100644
--- a/amaranth/sim/_pyrtl.py
+++ b/amaranth/sim/_pyrtl.py
@@ -166,11 +166,11 @@ class _RHSValueCompiler(_ValueCompiler):
             if value.operator == "%":
                 return f"zmod({sign(lhs)}, {sign(rhs)})"
             if value.operator == "&":
-                return f"({self(lhs)} & {self(rhs)})"
+                return f"({mask(lhs)} & {mask(rhs)})"
             if value.operator == "|":
-                return f"({self(lhs)} | {self(rhs)})"
+                return f"({mask(lhs)} | {mask(rhs)})"
             if value.operator == "^":
-                return f"({self(lhs)} ^ {self(rhs)})"
+                return f"({mask(lhs)} ^ {mask(rhs)})"
             if value.operator == "<<":
                 return f"({sign(lhs)} << {sign(rhs)})"
             if value.operator == ">>":


Test patch:
diff --git a/tests/test_sim.py b/tests/test_sim.py
index 6d38197..0cc8f50 100644
--- a/tests/test_sim.py
+++ b/tests/test_sim.py
@@ -927,3 +927,12 @@ class SimulatorRegressionTestCase(FHDLTestCase):
                 r"^Adding a clock process that drives a clock domain object named 'sync', "
                 r"which is distinct from an identically named domain in the simulated design$"):
             sim.add_clock(1e-6, domain=ClockDomain("sync"))
+
+    def test_bug_826(self):
+        sim = Simulator(Module())
+        def process():
+            self.assertEqual((yield C(0b0000, 4) | ~C(1, 1)), 0b0000)
+            self.assertEqual((yield C(0b1111, 4) & ~C(1, 1)), 0b0000)
+            self.assertEqual((yield C(0b1111, 4) ^ ~C(1, 1)), 0b1111)
+        sim.add_process(process)
+        sim.run()