Clash 1.9 update

[kleinreact]

This PR bumps Clash to the latest required 1.9 version (fixing the xpmCdcHandshake primitive) and applies the corresponding interface changes. In particular, this includes

• the correct usage of clockWizardDifferential getting rid of the improvised reset syncers,
• renaming of clock domains from BasicX to ExtX for all clocks that originate from a differential input, and
• changing the reset kind of all ExtX domains to Asynchronous resets.

---

Note that clockControlDemo0 currently does not compile due to a Clash bug introduced since the previously used version, which is under investigation right now.

[DigitalBrains1]

I think the difference between an improvised reset synchroniser with xpmCdcSingle and the bog-standard resetSynchronizer that is part of the safe PLL functions is that the former includes a max_delay constraint and an ASYNC_REG attribute whereas the latter includes none of those things. So I think the improvisation is qualitatively better...

[DigitalBrains1]

This is more a matter of preference, but

createDomain vXilinxSystem{vName="Ext125", vPeriod= hzToPeriod 125e6, vResetKind=Asynchronous}

is fully identical to

createDomain vSystem{vName="Ext125", vPeriod= hzToPeriod 125e6}

(or vIntelSystem instead of vSystem for maximum confusion ;-P)

[lmbollen]

So why the different records?

[DigitalBrains1]

Oof, good catch. This used to declare that the input and output clocks of the clock wizard were 100% identical, which means you could have just piped a signal synchronous to the input clock to the domain of the output clock or vice versa. But they are obviously not identical, so this might introduce metastability.

[DigitalBrains1]

(I have to admit the odds of you doing this accidentally are pretty low. But usually the type system prevents it and I really think you should not lie about equality of clocks even if the odds of it going wrong are low.)

[leonschoorl]

clash-lang/clash-compiler#2624 should allow clockControlDemo0 to build again

[martijnbastiaan]

@kleinreact Could you check whether @leonschoorl's patch works?

[kleinreact]

@kleinreact Could you check whether @leonschoorl's patch works?

Negative 😞. @leonschoorl: Is that new error related to your fixes?

[martijnbastiaan]

The output is confusing: it prints a warning (which we should be able to ignore if it doesn't come back later as an error). The real error is:

    Clash.Cores.Xilinx.DcFifo.Internal.BlackBoxes.dcFifoTF: dcFifo only supports synchronous resets

Do we connect dc fifo to an async reset? Because that would be an issue yeah...

[kleinreact]

Do we connect dc fifo to an async reset?

The only related change is the update of the external clock domains to use asynchronous resets, which is required for the safe variants of clock wizards and looks to me like the right way of doing it? So yes, if that's correct then we are connecting the dc fifo to an asynchronous reset somewhere in that instance.

[martijnbastiaan]

Hmmyeah, I was typing a response in another PR:

So in short, we don't have a good way of signalling "this is a signal that's unrelated to any (visible) domain".

[kleinreact]

But it still means we are using a "free reset" with dcFifo, which is unintended behavior, right? So the error indication points to a true error then?

[martijnbastiaan]

But it still means we are using a "free reset" with dcFifo, which is unintended behavior, right? So the error indication points to a true error then?

Yes, it is unintended behavior. The reset should be synchronized. The error just crops up for the "wrong" reasons.