search

clash-lang / clash-protocols

a battery-included library for dataflow protocols
Other
19 stars 7 forks source link

Handling unidirectional `Signal`s in circuits #59

Closed bgamari closed 7 months ago

bgamari commented 9 months ago

Currently clash-protocols provides the CSignal type for use in Circuits. This in principle allows us to write Circuits like (roughly modelling my axi-register package):

-- | An AXI4 subordinate implementing a memory-mapped register at the given Address.
-- (modelled after https://git.smart-cactus.org/ben/axi-register/-/blob/aca5748597c83d6e9d3d1c8e5f79b7390e3686e2/src/Protocols/Register.hs#L131) 
memoryMappedReg :: Address -> Circuit Axi4 (BitVector 32)
memoryMappedReg = ...

-- | Fan-out an AXI-MM bus; assumes that subordinates cover non-overlapping regions of address space.
-- (modelled after https://git.smart-cactus.org/ben/axi-register/-/blob/aca5748597c83d6e9d3d1c8e5f79b7390e3686e2/src/Protocols/Register.hs#L77)
fanoutAxi :: Circuit Axi4 (Axi4, Axi4)
fanoutAxi = ...

-- | A 'Circuit'  which produces the sum of two memory-mapped registers' values
sumRegs :: Circuit Axi4 (Signal dom (BitVector 32))
sumRegs = circuit $ \axi -> do
    (axi1, axi2) <- fanoutAxi -< axi
    Signal reg1 <- memoryMappedReg axi1
    Signal reg2 <- memoryMappedReg axi2
    idC -< Signal (reg1 + reg2)

Note that use of Signal patterns and expressions in sumRegs. As far as I can tell, this syntactic construct of circuit-notations allows the user to bind/use a unidirectional signal. However, I suspect that this currently doesn't work in clash-protocols as CSignal must be used in place of Signal in Circuits. My suspicion is that fixing this in the plugin should be quite straightforward.

Also see #58, which describes some confusion surrounding CSignal.

bgamari commented 9 months ago

I have opened https://github.com/cchalmers/circuit-notation/pull/17 addressing this.

martijnbastiaan commented 9 months ago

Does this actually compile? I think the plugin would handle Signal reg1 <- ... "correctly" as discussed in the PR, but I'm not sure how it would magically change CSignal to Signal.

bgamari commented 9 months ago

It doesn't. I realized that after opening it that upstream has shifted appreciably. Unfortunately it's not entirely clear to me how to account for these changes (particularly the addition of tagBundlePat, et al.) in clash-protocols.

martijnbastiaan commented 9 months ago

That PR went in before I could properly understand it. I didn't even realize it had downstream consequences :(.

martijnbastiaan commented 7 months ago

We've bumped to the latest circuit-notation in https://github.com/clash-lang/clash-protocols/pull/61. I think we've cleared up the confusion in the meantime. If that's incorrect please re-open the issue @bgamari.