clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.44k stars 154 forks source link

Example of a primitive in tutorial is outdated #2631

Open DigitalBrains1 opened 10 months ago

DigitalBrains1 commented 10 months ago

We have this example. But at a glance there are several things wrong:

  1. It matches on (Clock _) which does not type-check (we have a second argument these days).
  2. It uses NOINLINE. I don't think we should use CLASH_OPAQUE either; I think we should point out people should probably use a relatively recent GHC and OPAQUE for their own primitives. And perhaps point out that if they choose an old GHC nonetheless, they should use NOINLINE.
  3. We enhanced that specific primitive in a number of ways. We should decide whether we simplify the real code, present the real code or present a different primitive altogether to keep the discussion focussed. One of the enhancements that stands out to me is that we reduced the number of formal arguments to improve simulation performance. We also improved handling of XExceptions a lot.

Point 2 will apply to more documentation, by the way.

JvWesterveld commented 4 months ago

I wasn't yet aware of the new OPAQUE GHC pragma, and seeing CLASH_OPAQUE here confused me for a bit before I found the PR introducing it. Apparently, CLASH_OPAQUE is specific to code in the clash-compiler repo, which includes clash-cores.

I agree that relevant documentation (e.g. on the use of Synthesize annotations (1, 2) and primitives) should be updated to mention the existence of OPAQUE and when one would want to use it with it apparently being preferred over NOINLINE with GHC ≥9.4. On a related note, perhaps the documentation should also allude to the existence of the new-ish way to write blackboxes?