`condition` unexpected behavior

[lekcyjna123]

Hi @tilk , today I played a little bit with conditions to introduce some syntax sugar and I have found that defining two conditions in the same method both with priority=True introduce a cycle in conflict graph. But I can not find an explanation fo that. Is this expected behaviour?

To reproduce the issue: In ConditionTestCircuit in test_transaction_lib.py use instead of trunk source method following definition (simply a second condition is added):

        @def_method(m, self.source)
        def _(cond1, cond2, cond3):
            with condition(m, nonblocking=self.nonblocking, priority=self.priority) as branch:
                with branch(cond1):
                    self.target(m, cond=1)
                with branch(cond2):
                    self.target(m, cond=2)
                with branch(cond3):
                    self.target(m, cond=3)
                if self.catchall:
                    with branch():
                        self.target(m, cond=0)
            with condition(m, priority = True) as branch:
                with branch(1):
                    pass

After that test fails with error:

======================================================================
ERROR: test_condition_7 (test.transactions.test_transaction_lib.ConditionTest.test_condition_7)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kuba/.local/lib/python3.11/site-packages/parameterized/parameterized.py", line 533, in standalone_func
    return func(*(a + p.args), **p.kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/DANE/Dokumenty/Informatyka/Verilog/Kuznia/coreblocks/test/transactions/test_transaction_lib.py", line 802, in test_condition
    with self.run_simulation(m) as sim:
  File "/usr/lib/python3.11/contextlib.py", line 137, in __enter__
    return next(self.gen)
           ^^^^^^^^^^^^^^
  File "/mnt/DANE/Dokumenty/Informatyka/Verilog/Kuznia/coreblocks/test/common.py", line 482, in run_simulation
    sim = PysimSimulator(
          ^^^^^^^^^^^^^^^
  File "/mnt/DANE/Dokumenty/Informatyka/Verilog/Kuznia/coreblocks/test/common.py", line 434, in __init__
    super().__init__(TestModule(module, add_transaction_module))
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/sim/core.py", line 67, in __init__
    self._fragment = Fragment.get(fragment, platform=None).prepare()
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/hdl/ir.py", line 37, in get
    new_obj = obj.elaborate(platform)
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/hdl/dsl.py", line 537, in elaborate
    fragment.add_subfragment(Fragment.get(self._named_submodules[name], platform), name)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/hdl/ir.py", line 37, in get
    new_obj = obj.elaborate(platform)
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/hdl/dsl.py", line 537, in elaborate
    fragment.add_subfragment(Fragment.get(self._named_submodules[name], platform), name)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kuba/.local/lib/python3.11/site-packages/amaranth/hdl/ir.py", line 37, in get
    new_obj = obj.elaborate(platform)
              ^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/DANE/Dokumenty/Informatyka/Verilog/Kuznia/coreblocks/coreblocks/transactions/core.py", line 422, in elaborate
    cgr, rgr, porder = TransactionManager._conflict_graph(method_map, relations)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/DANE/Dokumenty/Informatyka/Verilog/Kuznia/coreblocks/coreblocks/transactions/core.py", line 274, in _conflict_graph
    for k, transaction in enumerate(TopologicalSorter(pgr).static_order()):
  File "/usr/lib/python3.11/graphlib.py", line 244, in static_order
    self.prepare()
  File "/usr/lib/python3.11/graphlib.py", line 106, in prepare
    raise CycleError(f"nodes are in a cycle", cycle)
graphlib.CycleError: ('nodes are in a cycle', [(transaction source_cond1_AdapterTrans_source_source_cond1), (transaction source_cond0_AdapterTrans_source_source_cond2), (transaction source_cond3_AdapterTrans_source_source_cond0), (transaction source_cond1_AdapterTrans_source_source_cond1)])

[tilk]

First, your reproduction example doesn't trigger the issue, which is obvious: you included only one branch in the second condition.

Second: yes, this is the consequence of the design. Suppose you have two priority conditions: first with branches A, B, and second with branches C, D. Therefore, A < B and C < D. Because of how simultaneous transactions work, you get four transactions: AC, AD, BC, and BD, and the orderings transfer to them. So, because A < B, AD < BC, but because C < D, BC < AD.

[lekcyjna123]

Thanks for explanation. Now I understand.

[tilk]

Also, a word of caution: priority conditions kinda look like if-else, but don't really work like that. In particular, a later branch can be selected if a previous one is blocked. It only guarantees that, if two branches can be executed in a given clock cycle, the first one in code order will be selected. The underlying mechanism is transaction ordering.

[lekcyjna123]

First, your reproduction example doesn't trigger the issue, which is obvious: you included only one branch in the second condition.

I have checked and it looks like my reproduction example doesn't work on newest master. But on commits before #415 my reproduction example works and an error is being thrown. On newest branch to the second condition nonblocking=True has to be added.

Ach, and that is expected because nonblocking=True add a second transaction so there is a conflict.