Add Xilinx dual clock FIFO primitive

[martijnbastiaan]

This PR adds a Xilinx dual clock FIFO implementation to clash-cores.

Comments for implementers:

• There's no observable difference between the different "implementation mode"s AFAIK. Feel free to ignore for the Haskell model.
• We don't care too much about having the Haskell model correspond 1:1 to the RTL for our client's project. Please de-prioritize.

How to test

The easiest way to create a quick test bench (IMO) is to instantiate a new Clash project with stack new fifotest clash-lang/simple, add

../clash-compiler/clash-cores/clash-cores.cabal
../clash-compiler/clash-prelude/clash-prelude.cabal
../clash-compiler/clash-lib/clash-lib.cabal
../clash-compiler/clash-ghc/clash-ghc.cabal

to cabal.project and run cabal run clash -- Test.hs --vhdl -fclash-clear -fclash-no-cache && cat vhdl/Test.topEntity/dcfifo* where Test.hs is something like:

module Test where

import Clash.Prelude

import Clash.Cores.Xilinx.DcFifo.Explicit

type WriteDom = System
type ReadDom = System

topEntity ::
  Clock WriteDom -> Clock ReadDom -> Reset ReadDom ->

  -- | Write data
  Signal WriteDom (BitVector 32) ->
  -- | Write enable
  Signal WriteDom Bool ->
  -- | Read enable
  Signal ReadDom Bool ->
  -- |
  ( Signal WriteDom ResetBusy
  , Signal WriteDom Full
  , Signal WriteDom (DataCount 5)

  , Signal ReadDom ResetBusy
  , Signal ReadDom Empty
  , Signal ReadDom (DataCount 5)
  , Signal ReadDom (BitVector 32)
  )
topEntity wClk rClk rRst = dcFifo defConfig{dcReadDataCount=False} wClk rClk rRst

Questions for reviewers:

• Dual clock FIFOs and single clock FIFOs differ enough in terms of signals/features that I've decided to preemptively namespace the dual clock FIFOs. All function names are still prefixed with dc though. Remove?
• I've instantiated a very specific version of fifo_generator here (13.2) - pretty much linked to Vivado 2021.2. How can we deal with versioning?

Resources:

• FIFO Generator v13.2 (PDF)

Still TODO:

• Add Haskell model (@vmchale)
• Add Haskell tests (@vmchale)
• Write documentation (@vmchale, @martijnbastiaan)
• Write checkDcConfig :: DcConfig -> Maybe Error that returns a (human-readable) error message for misconfigurations. not relevant anymore
• Make primitive return booleans (we probably want to implement boolFromBit analogous to boolToBit, right now it can only be used in VHDL) (@vmchale)
• Add RTL tests (@vmchale, @DigitalBrains1?)
• Made primitive a multi result primitive for prettier HDL (@vmchale / @martijnbastiaan) Attempting this triggers a bug, I'll post an issue.
• Make Clash.Cores.Xilinx.Floating use functions in Clash.Cores.Xilinx.Common? not for this PR
• Check copyright notices are up to date in edited files
• Write a changelog entry (see changelog/README.md) clash-cores isn't released on Hackage

[martijnbastiaan]

Here's some inspiration for a dual clock test bench:

https://github.com/clash-lang/clash-compiler/blob/master/tests/shouldwork/Signal/DualBlockRam0.hs

Given that we don't have a cycle accurate Haskell model, you'd probably want to build a circuit similar to the one you've got in the unittests, and test your assumptions in HDL.

[DigitalBrains1]

I've instantiated a very specific version of fifo_generator here (13.2) - pretty much linked to Vivado 2021.2. How can we deal with versioning?

Looking at the revision history in the product guide, 13.2 has been the version since 2017-10-04. I doubt you can reasonably deal with changes to the FIFO core; that seems to quickly lead to separate Clash functions or modules for separate Vivado versions. How would you deal with a new configuration option, new signals in Clash? One Vivado version can generally generate only a single version of bundled IP; it seems simplest to just pin a version of clash-cores to a version range of Vivado. We'd have to watch out that we don't generate one IP for one version and the other IP for another version.

In general, I suggest tracking the latest version of Vivado available for all platforms. If people really need an old version of Vivado, it might be more of a hassle for them.

Vivado can upgrade an older version of IP to a (reasonably-identically configured?) new version, though. So if Vivado switches to 13.3, people can still synthesise by introducing another step in their workflow: using Vivado to upgrade the IP. It seems they just need to add this to their Tcl:

upgrade_ip [get_ips *]

https://www.xilinx.com/products/intellectual-property/vivado-ip-versioning.html

[martijnbastiaan]

Vivado can upgrade an older version of IP to a (reasonably-identically configured?) new version, though. So if Vivado switches to 13.3, people can still synthesise by introducing another step in their workflow: using Vivado to upgrade the IP. It seems they just need to add this to their Tcl:

Cool @DigitalBrains1, I've seen that before, it should just work. Another option would be to add the version to the DcConfig. This would let users override the version in case of need. Obviously, this would be dangerous due to possible API changes but that's on the user. Perhaps we could even test for versions in the BlackBox 🤔. We could make an ADT to make it explicit that you're doing something unsupported:

data Version
  = v13_3
  | v13_4
  | CustomVersion String

[martijnbastiaan]

These signals are generated as wire, but should be generated as reg:

  reg  wr_full_bool;
  reg  rd_empty_bool;
  reg  wr_enable;
  reg  rd_enable;
  reg  wr_reset_busy_bool;
  reg  rd_reset_busy_bool;

Edit: done

[martijnbastiaan]

@vmchale Could you split this up into three PRs:

1. Support VHDL component declaration in blocks
2. Support Vivado in CI
3. New: Xilinx DCFifo (this PR has become a bit cluttered)

I've already started with (2) as I wanted to get Vivado running on CI:

• https://github.com/clash-lang/clash-compiler/commits/add-vivado-support

That branch has CI setup with Vivado running on it. It's probably best to comment out some GHC versions in .gitlab.yml to save on CI resources while testing that. Enabling Vivado for all tests does lead some weird cycles:

UnkindPartition/tasty#340

so it probably takes some debugging..

[vmchale]

The PR for component declarations in blocks is here: #2255

I can rebase on that once it is merged.

[vmchale]

This is based on #2257 but CI is passing now.

[vmchale]

I think this is also going to need to be squashed :(

[DigitalBrains1]

I think this is also going to need to be squashed :(

Martijn suggested above that you create a new PR for this. Anyway, I think this can be a single commit indeed, but we'll see in review.

[vmchale]

Oh you're right @DigitalBrains1! I'll close this; wait til the Vivado support is merged so it can be sensibly reviewed.