Fix VHDL and SystemVerilog gen for Xilinx oddr

[DigitalBrains1]

Verilog correctly picked one of the possible arguments to inspect for ResetKind, but the others picked the KnownNat constraint.

This lead to the following error message:

<no location info>: error:
    Clash error call:
    Don't know how to get a Domain out of HWType: Unsigned 64
    CallStack (from HasCallStack):
      error, called at src/Clash/Netlist/BlackBox/Util.hs:654:8 in clash-lib-1.9.0-inplace:Clash.Netlist.BlackBox.Util
      generalGetDomainConf, called at src/Clash/Netlist/BlackBox/Util.hs:639:17 in clash-lib-1.9.0-inplace:Clash.Netli
st.BlackBox.Util
      getDomainConf, called at src/Clash/Netlist/BlackBox/Util.hs:597:20 in clash-lib-1.9.0-inplace:Clash.Netlist.Blac
kBox.Util
      renderElem, called at src/Clash/Netlist/BlackBox/Util.hs:320:15 in clash-lib-1.9.0-inplace:Clash.Netlist.BlackBo
x.Util

Without Vivado, we can't actually test these primitives. However, here they already errored out during HDL generation. Perhaps we should add a "HDL generation only" test for all primitives that don't yet occur in any of the other tests in the test suite. This would have caught this particular bug.

The bug was introduced in 2019 in commit 0c38d65 part of PR #527.

Still TODO:

• Write a changelog entry (see changelog/README.md)
• Check copyright notices are up to date in edited files

[DigitalBrains1]

Without Vivado, we can't actually test these primitives.

Of course, while we didn't have Vivado back when the bug was introduced, we do now. Maybe it's even worth it to make a combined test with IDDR and ODDR prims, but just a "does this generate HDL" test is much more quickly written.

Anyway, the names in HDL are a bit weird:

        D1 => dataout_l(i),    -- 1-bit data input (positive edge)
        D2 => dataout_h(i),    -- 1-bit data input (negative edge)

Why is the data for the positive edge labeled L and the data for the negative edge labeled H? Note that Clash.Explicit.DDR.ddrOut labels them data_Pos and data_Neg respectively, which makes more sense.

[christiaanb]

I think they're called like that due to copy/paste from the Intel primitives where the output ports of the Intel primivite are called dataout_l and dataout_h:

clash-compiler/clash-lib/prims/vhdl/Clash_Intel_DDR.primitives.yaml

Lines 43 to 44 in 5706eaf

	dataout_h => ~SYM[2],
	dataout_l => ~SYM[1]

[DigitalBrains1]

Shall I name them more obviously in this PR as well? I'm thinking dataout_p and dataout_n data_pos and data_neg, after the edge that they are presented. It's similar to what Clash.Explicit.DDR.ddrOut does, but without capitalisation. I think we more regularly use underscores as word separators in variables in HDL, and to use both underscores as well as case to separate words feels redundant.

[christiaanb]

Yes, please clean up the names, and also it would be nice if we tested these primitives on Vivado at least in nightly builds. I mean... we already do that for the generic DDRout primitives:

clash-compiler/tests/Main.hs

Lines 517 to 526 in 10f26ff

	, let _opts = def{ buildTargets = BuildSpecific [ "testBenchUA"
	, "testBenchUS"
	, "testBenchGA"
	, "testBenchGS"
	]
	, hdlLoad = [Vivado]
	, hdlSim = [Vivado]
	, clashFlags=["-fclash-hdlsyn", "Vivado"]
	}
	in runTest "DDRout" _opts

[DigitalBrains1]

Note to self: to do: run the nightly tests on a temporary branch to be absolutely sure they did not break. The changed test benches were run succesfully under a single configuration on my laptop, but CI will test a whole bunch of configurations.