Allow multiple `with m.State("..."):` statements for the same state #195

RobertBaruch · 2019-09-03T04:29:45Z

If I have an FSM that has a bunch of named states, I'd like a convenient way to do something like this:

        tcycles = {
            "INACTIVE": 0,
            "TCYCLE1": 1,
            "TCYCLE2": 2,
            "TCYCLE3": 3,
            "TCYCLE4": 4,
            "TCYCLE5": 5
        }

        with m.FSM(domain="pos", reset="INACTIVE") as fsm:
            tcycle = tcycles[fsm.state]
            m.d.comb += self.A.eq([0, s1, s1, s2, s2, ~s2][tcycle])

What's the idiomatic way to do this?

The text was updated successfully, but these errors were encountered:

whitequark · 2019-09-03T04:31:41Z

It's hard for me to suggest something truly idiomatic without seeing a wider picture here. Can you post the entire module, or at least sketch it out?

RobertBaruch · 2019-09-03T14:37:27Z

Here's what I have so far. "Edgelord" is a module that reproduces the clk state for use in combinatorial logic, without subjecting the clk signal to combinatorial logic. The goal here is to provide the output signals as a readable table, rather than scattered throughout the FSM.

from nmigen import *
from nmigen.cli import main
from nmigen.asserts import *
from nmigen.ast import SignalDict
from edgelord import Edgelord


class M1(Elaboratable):
    def __init__(self):
        self.activate = Signal()
        self.pc = Signal(16)
        self.refresh_addr = Signal(16)
        self.D = Signal(8)
        self.nWAIT = Signal()
        self.extend_cycle = Signal()

        self.A = Signal(16)
        self.nMREQ = Signal()
        self.nRD = Signal()
        self.nM1 = Signal()
        self.nRFSH = Signal()
        self.rdata = Signal(8)
        self.tcycle = Signal(3)
        self.extra_tcycle = Signal()
        self.done = Signal()

        self.ports = [
            self.activate, self.pc, self.refresh_addr, self.D, self.nWAIT,
            self.extend_cycle, self.A, self.nMREQ, self.nRD, self.nM1,
            self.nRFSH, self.rdata, self.tcycle, self.extra_tcycle, self.done
        ]

    def elaborate(self, platform):
        m = Module()
        m.submodules.edgelord = edgelord = Edgelord()

        waitstated = Signal()
        m.d.neg += waitstated.eq(self.nWAIT)

        tcycles = {
            "INACTIVE": 0,
            "TCYCLE1": 1,
            "TCYCLE2": 2,
            "TCYCLE3": 3,
            "TCYCLE4": 4,
            "TCYCLE5": 5
        }

        pc = self.pc
        ra = self.refresh_addr
        c = edgelord.clk_state
        A = Array((0, pc, pc, ra, ra, ra))

        with m.FSM(domain="pos", reset="INACTIVE") as fsm:
            tcycle = tcycles[fsm.state]
            m.d.comb += self.tcycle.eq(tcycle)
            m.d.comb += self.extra_tcycle(self.extend_cycle and tcycle >= 4)
            m.d.comb += self.A.eq([0, pc, pc, ra, ra, ra][tcycle])
            m.d.comb += self.nMREQ.eq([1, c, 0, c, ~c, 1][tcycle])
            m.d.comb += self.nRD.eq([1, c, 0, 1, 1, 1][tcycle])
            m.d.comb += self.nM1.eq([1, 0, 0, 1, 1, 1][tcycle])
            m.d.comb += self.nRFSH.eq([1, 1, 1, 0, 0, 1][tcycle])
            m.d.comb += self.done.eq([0, 0, 0, 0, ~c, 1][tcycle])

            with m.State("INACTIVE"):
                m.d.pos += self.rdata.eq(0)
                with m.If(self.activate):
                    m.next = "TCYCLE1"

            with m.State("TCYCLE1"):
                m.next = "TCYCLE2"

            with m.State("TCYCLE2"):
                with m.If(~waitstated):
                    m.d.pos += self.rdata.eq(self.D)
                    m.next = "TCYCLE3"

            with m.State("TCYCLE3"):
                m.next = "TCYCLE4"

            with m.State("TCYCLE4"):
                with m.If(self.extend_cycle):
                    m.next = "TCYCLE5"
                with m.Elif(self.activate):
                    m.next = "TCYCLE1"
                with m.Else():
                    m.next = "INACTIVE"

            with m.State("TCYCLE5"):
                with m.If(self.activate):
                    m.next = "TCYCLE1"
                with m.Else():
                    m.next = "INACTIVE"

        return m


if __name__ == "__main__":
    clk = Signal()
    rst = Signal()

    pos = ClockDomain()
    pos.clk = clk
    pos.rst = rst

    neg = ClockDomain(clk_edge="neg")
    neg.clk = clk
    neg.rst = rst

    m1 = M1()

    m = Module()
    m.domains.pos = pos
    m.domains.neg = neg
    m.submodules.m1 = m1

    main(m, ports=[clk, rst] + m1.ports, platform="formal")

whitequark · 2019-09-03T21:24:38Z

Try this:

        tcycles = {
            "INACTIVE": 0,
            "TCYCLE1": 1,
            "TCYCLE2": 2,
            "TCYCLE3": 3,
            "TCYCLE4": 4,
            "TCYCLE5": 5
        }
        with m.FSM(domain="pos", reset="INACTIVE") as fsm:
            tcycle = Signal(max=len(tcycles))
            for state, index in tcycles.items():
                with m.State(state):
                    m.d.comb += tcycle.eq(index)
            m.d.comb += self.tcycle.eq(tcycle)
            m.d.comb += self.extra_tcycle(self.extend_cycle and tcycle >= 4)
            m.d.comb += self.A.eq(Array([0, pc, pc, ra, ra, ra])[tcycle])
            m.d.comb += self.nMREQ.eq(Array([1, c, 0, c, ~c, 1])[tcycle])
            m.d.comb += self.nRD.eq(Array([1, c, 0, 1, 1, 1])[tcycle])
            m.d.comb += self.nM1.eq(Array([1, 0, 0, 1, 1, 1])[tcycle])
            m.d.comb += self.nRFSH.eq(Array([1, 1, 1, 0, 0, 1])[tcycle])
            m.d.comb += self.done.eq(Array([0, 0, 0, 0, ~c, 1])[tcycle])

whitequark · 2019-09-03T21:25:22Z

Note that you can have as many m.State statements as you want, they act as if they were all concatenated together in execution order. (Best not to have overlapping side effects, anyhow.)

RobertBaruch · 2019-09-04T03:46:07Z

Seems reasonable. However, this line is causing a problem, and I'm not sure which value it's having a problem with:

  File "m1.py", line 64, in elaborate
    m.d.comb += self.extra_tcycle(self.extend_cycle and tcycle >= 4)
  File "/home/robertbaruch/.local/lib/python3.6/site-packages/nmigen/hdl/ast.py", line 49, in __bool__
    raise TypeError("Attempted to convert nMigen value to boolean")

None of these worked either:

#    m.d.comb += self.extra_tcycle(self.extend_cycle and (tcycle >= 4))
#    m.d.comb += self.extra_tcycle(self.extend_cycle == 1 and tcycle >= 4)
#    m.d.comb += self.extra_tcycle(self.extend_cycle == 1 & tcycle >= 4)
#       ^ This one resulted in "TypeError: 'Signal' object is not callable"

whitequark · 2019-09-04T03:52:41Z

What are you trying to do with self.extra_tcycle exactly? I mean, signals aren't callable--what do you expect will happen if you do call it?

whitequark · 2019-09-04T03:53:25Z

Oh... did you miss a .eq? Should it be something like

m.d.comb += self.extra_tcycle.eq((self.extend_cycle == 1) & (tcycle >= 4))

(note the parens, necessary because of precedence)

RobertBaruch · 2019-09-04T04:05:37Z

Oops, my bad.

Next issue:

  File "m1.py", line 73, in elaborate
    with m.State("INACTIVE"):
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/home/robertbaruch/.local/lib/python3.6/site-packages/nmigen/hdl/dsl.py", line 325, in State
    raise SyntaxError("FSM state '{}' is already defined".format(name))
nmigen.hdl.dsl.SyntaxError: FSM state 'INACTIVE' is already defined

I was kind of hoping that I could define the table of outputs in one place, and the transitions in another, but we can't have two with m.State with the same name...

whitequark · 2019-09-04T04:08:49Z

I was kind of hoping that I could define the table of outputs in one place, and the transitions in another, but we can't have two with m.State with the same name...

I agree. I'll look into relaxing this restriction (I thought it's already relaxed...) but this will take a bit of time (the lazy state definition interacts with unspecified reset state, and should be handled in a nice way), so you should use a workaround for now.

RobertBaruch · 2019-09-04T04:09:21Z

Got it, thanks!

whitequark · 2019-09-14T21:22:43Z

Closing in favor of more general discussion in #206.

whitequark added the question label Sep 3, 2019

whitequark changed the title ~~Mapping FSM state to integer, then as index in array of signals?~~ Allow multiple with m.State("..."): statements for the same state Sep 4, 2019

whitequark added feature and removed question labels Sep 4, 2019

whitequark added question and removed feature labels Sep 14, 2019

whitequark closed this as completed Sep 14, 2019

nmigen-issue-migration mentioned this issue Jan 27, 2020

Allow multiple with m.State("..."): statements for the same state amaranth-lang/amaranth#196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple `with m.State("..."):` statements for the same state #195

Allow multiple `with m.State("..."):` statements for the same state #195

RobertBaruch commented Sep 3, 2019

whitequark commented Sep 3, 2019

RobertBaruch commented Sep 3, 2019 •

edited

whitequark commented Sep 3, 2019

whitequark commented Sep 3, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 4, 2019

whitequark commented Sep 4, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 4, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 14, 2019

Allow multiple with m.State("..."): statements for the same state #195

Allow multiple with m.State("..."): statements for the same state #195

Comments

RobertBaruch commented Sep 3, 2019

whitequark commented Sep 3, 2019

RobertBaruch commented Sep 3, 2019 • edited

whitequark commented Sep 3, 2019

whitequark commented Sep 3, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 4, 2019

whitequark commented Sep 4, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 4, 2019

RobertBaruch commented Sep 4, 2019

whitequark commented Sep 14, 2019

Allow multiple `with m.State("..."):` statements for the same state #195

Allow multiple `with m.State("..."):` statements for the same state #195

RobertBaruch commented Sep 3, 2019 •

edited