TestBench annotation confusion

[leonschoorl]

There seems to be some confusion about the direction of the TestBench annotation.
Is it:

1. an annotation on a Synthesized binder pointing to its testbench
2. an annotation on a testbench pointing to the thing that it is testing

The current docs state:

Tell what binder is the TestBench for a Synthesize-annotated binder.

Not very clear.
But luckily there is also an example:

..., f has a Synthesize annotation, and g is the HDL test bench for f.

f = ...
{-# ANN f (defSyn "f") #-}
{-# ANN f (TestBench 'g) #-}

g :: Signal Bool
g = ...

This is option 1.
This work, and clash will create HDL output for both f and g, but it will produce a warning:

Clash: Compiling TestNames.g
Clash: Normalization took 0.001s
[WARNING] Dubious primitive instantiation for Clash.Explicit.Testbench.tbClockGen: Clash.Signal.Internal.tbClockGen is not synthesizable! (disable with -fclash-no-prim-warn)
Clash: Netlist generation took 0.000s
Clash: Compiling TestNames.f
Clash: Normalization took 0.001s
Clash: Netlist generation took 0.000s

If we try option 2:

f = ...
{-# ANN f (defSyn "f") #-}

g :: Signal Bool
g = ...
{-# ANN g (TestBench 'f) #-}

This also work, clash will also generate HDL files for both f and g, but this time the netlist code actually considers g to be a testbench and suppresses the tbClockGen warning:

Clash: Compiling TestNames.g
Clash: Normalization took 0.001s
Clash: Netlist generation took 0.000s
Clash: Compiling TestNames.f
Clash: Normalization took 0.001s
Clash: Netlist generation took 0.000s
Clash: Total compilation took 0.957s

[martijnbastiaan]

Good points. I've always interpreted it as (2) and used that for implementations.

At least in the current implementation there's only one reason to use a test bench annotation: to suppress warnings of dubious blackbox instantiations. Another feature could be (which is currently not implemented) that -main-is f would also compile all of f's test benches.

My proposal would be to entirely eliminate the testbench annotation. Instead, we could add a field Synthesize.isTestbench which would disable the aforementioned warning. I'm not convinced the second benefit I mentioned is really worth the effort. We could replace it with the ability to do -main-is=f,g though.

[DigitalBrains1]

Currently, a

{-# ANN myTestBench (TestBench 'entityBeingTested) #-}

annotation in a sense "implies" a Synthesize annotation on entityBeingTested, so if we adopt the Synthesize.isTestBench approach, the semantics slightly change. We need to account for that in a few test benches I wrote in clash-testsuite, and users need to be made aware.