CDC primitives are particularly troublesome for synthesizers because they interact badly with features like retiming. Because of this, toolchains require them to be marked in some way, often by instantiating them in a specific way, adding attributes, etc.

This was handled in oMigen in a particularly awkward way, essentially privileging ISE and Vivado above any other toolchain, and tailoring the generic primitives towards them as well. (The attr_translate mechanism looks generic, but in practice it was only barely flexible enough to support these two toolchains.)

In this issue I'd like to collect toolchain requirements so that a more flexible implementation can be done, one that would not be tied to Xilinx.

The following are the uses of attr.add and attr= in oMigen, assuming they are the only places where such attributes originate:

• In MultiRegImpl, no_retiming is added on internal registers.
• In BusSynchronizer, something similar is done to the buffer. (Can this be done in terms of MultiReg?)
• In XilinxMultiRegImpl, mr_ff, async_reg and no_shreg_extract are added on internal registers.
• In XilinxAsyncResetSynchronizerImpl, ars_ff1 and ars_ff2 are added on the internal registers.

Here is my overall proposal:

• The generic lib.cdc primitives should not make any attempt to influence toolchain behavior. In particular, explicitly marked CDC (Require signals crossing clock domains to be explicitly marked #4) should be required and sufficient to inhibit retiming across CDC primitives. (With the design proposed in Require signals crossing clock domains to be explicitly marked #4, only the first flop in a chain will be marked as performing CDC. This is a problem for extracting no_retiming attributes that needs to be solved.)
• (Aside: since RTLIL can directly represent async flops, nMigen's [Async]ResetSynchronizer in principle does not have to be lowered to a platform-specific implementation.)
• Each toolchain that has special requirements for MultiReg, ResetSynchronizer and possibly other primitives, should make sure they are lowered to an appropriate platform-specific implementation. Once that happens, it is free to add false path, etc, constraints in any way it considers appropriate.

@sbourdeauducq @jordens Feedback? Objections?

Beyond that I'd like to read reference material, particularly from Xilinx, explaining exactly how to add constraints to CDC primitives. In particular the Vivado implementation seems complex and perhaps overly fragile, with the hardcoded 2ns delta:

    def _constrain(self, platform):
        # The asynchronous input to a MultiReg is a false path
        platform.add_platform_command(
            "set_false_path -quiet "
            "-to [get_nets -filter {{mr_ff == TRUE}}]"
        )
        # The asychronous reset input to the AsyncResetSynchronizer is a false
        # path
        platform.add_platform_command(
            "set_false_path -quiet "
            "-to [get_pins -filter {{REF_PIN_NAME == PRE}} "
                "-of [get_cells -filter {{ars_ff1 == TRUE || ars_ff2 == TRUE}}]]"
        )
        # clock_period-2ns to resolve metastability on the wire between the
        # AsyncResetSynchronizer FFs
        platform.add_platform_command(
            "set_max_delay 2 -quiet "
            "-from [get_pins -filter {{REF_PIN_NAME == Q}} "
                "-of [get_cells -filter {{ars_ff1 == TRUE}}]] "
            "-to [get_pins -filter {{REF_PIN_NAME == D}} "
                "-of [get_cells -filter {{ars_ff2 == TRUE}}]]"
        )