DDR: Improve API, document

The existing constraint can be redefined in terms of `DomainPeriod`.
Since the virtual DDR domain only exists in simulation, it really
doesn't matter if it doesn't match with the real domain in all its
parameters, so we can drop those and make the constraints more intuitive
to read.

The primitives for Xilinx and Intel only support the rising edge as the
active edge of the domain.

The real domain of the functions was the `slow` domain, and the `fast`
domain was the virtual domain. The new names `dom` and `domDDR`
reinforce the fact that `dom` is the real domain.

Author: DigitalBrains1

changelog/2024-12-14T13_35_37+01_00_fix_ddr_prims

@@ -2,8 +2,10 @@ FIXED: `Clash.Explicit.DDR`:
   - `ddrIn`: VHDL: Remove data input from sensitivity list of `ddrIn_neg_latch` register as it is superfluous. This should not affect functionality.
   - `ddrOut`: VHDL: Fix incorrect usage of `Enable` input when the domain is set to asynchronous resets. Deasserting the `Enable` exhibited wrong behavior before this fix.
 FIXED: `Clash.Xilinx.DDR`:
+  - These primitives only support clocks where the rising edge is the active edge. Using them in a domain with falling active edges now causes an error.
   - `oddr`: Fix VHDL and SystemVerilog erroring out during HDL generation
   - Symbols in HDL for both `iddr` and `oddr` were renamed to match their function.
 FIXED: `Clash.Intel.DDR`:
+  - These primitives only support clocks where the rising edge is the active edge. Using them in a domain with falling active edges now causes an error.
   - Fix rendering HDL. It variously errored out or generated non-working HDL.
   - Rendering HDL no longer causes Clash to issue a warning about an argument unused in Haskell but used in the primitive black box.

changelog/2025-01-22T16_23_00+01_00_improve_ddr

@@ -0,0 +1,4 @@
+CHANGED: `Clash.Explicit.DDR`, `Clash.Intel.DDR` and `Clash.Xilinx.DDR`:
+  - The constraints for the functions have been rewritten to use `DomainPeriod`, which makes them more readable and relaxes unnecessary constraints on the virtual DDR domain. The type-level variables for the domains have been renamed. `dom` is a real domain, `domDDR` is the virtual DDR domain.
+  - The Xilinx and Intel primitives only support domains where the rising edge is the active edge. This is now enforced at the type level by adding a constraint.
+  - The Xilinx and Intel primitives now directly support any data type that has a `BitPack` instance.

clash-lib/prims/commonverilog/Clash_Intel_DDR.primitives.yaml

@@ -5,39 +5,42 @@
     - altera_mf
     type: |-
       altddioOut#
-        :: ( HasCallStack             -- ARG[0]
-           , KnownConfi~ fast domf    -- ARG[1]
-           , KnownConfi~ slow doms    -- ARG[2]
-           , KnownNat m )             -- ARG[3]
-        => SSymbol deviceFamily       -- ARG[4]
-        -> Clock slow                 -- ARG[5]
-        -> Reset slow                 -- ARG[6]
-        -> Enable slow                -- ARG[7]
-        -> Signal slow (BitVector m)  -- ARG[8]
-        -> Signal slow (BitVector m)  -- ARG[9]
-        -> Signal fast (BitVector m)
+        :: forall deviceFamily n dom domDDR
+         . HasCallStack              -- ARG[0]
+        => KnownDomain dom           -- ARG[1]
+        => KnownDomain domDDR        -- ARG[2]
+        => DomPeriod ~ 2 * ...       -- ARG[3]
+        => DomEdge ~ Rising          -- ARG[4]
+        => KnownNat n                -- ARG[5]
+        => SSymbol deviceFamily      -- ARG[6]
+        -> Clock dom                 -- ARG[7]
+        -> Reset dom                 -- ARG[8]
+        -> Enable dom                -- ARG[9]
+        -> Signal dom (BitVector n)  -- ARG[10]
+        -> Signal dom (BitVector n)  -- ARG[11]
+        -> Signal domDDR (BitVector n)
     template: |-
       // altddioOut begin
       altddio_out
         #(
           .extend_oe_disable ("OFF"),
-          .intended_device_family (~LIT[4]),
+          .intended_device_family (~LIT[6]),
           .invert_output ("OFF"),
           .lpm_hint ("UNUSED"),
           .lpm_type ("altddio_out"),
           .oe_reg ("UNREGISTERED"),
           .power_up_high ("OFF"),
           .width (~SIZE[~TYPO])
         )
-        ~GENSYM[~COMPNAME_ALTDDIO_OUT][7] (~IF ~ISSYNC[2] ~THEN
-          .sclr (~ARG[6]),
+        ~GENSYM[~COMPNAME_ALTDDIO_OUT][7] (~IF ~ISSYNC[1] ~THEN
+          .sclr (~ARG[8]),
           .aclr (1'b0),~ELSE
-          .aclr (~ARG[6]),
+          .aclr (~ARG[8]),
           .sclr (1'b0),~FI
-          .datain_h (~ARG[8]),
-          .datain_l (~ARG[9]),
-          .outclock (~ARG[5]),
-          .outclocken (~IF ~ISACTIVEENABLE[7] ~THEN ~ARG[7] ~ELSE 1'b1 ~FI),
+          .datain_h (~ARG[10]),
+          .datain_l (~ARG[11]),
+          .outclock (~ARG[7]),
+          .outclocken (~IF ~ISACTIVEENABLE[9] ~THEN ~ARG[9] ~ELSE 1'b1 ~FI),
           .dataout (~RESULT),
           .aset (1'b0),
           .sset (1'b0),

clash-lib/prims/systemverilog/Clash_Explicit_DDR.primitives.yaml

@@ -2,42 +2,43 @@
     name: Clash.Explicit.DDR.ddrIn#
     kind: Declaration
     type: |-
-      ddrIn# :: forall a slow fast n pFast gated synchronous.
-                 ( HasCallStack          -- ARG[0]
-                 , NFDataX a             -- ARG[1]
-                 , KnownConfi~ fast domf -- ARG[2]
-                 , KnownConfi~ slow doms -- ARG[3]
-              => Clock slow              -- ARG[4], clk
-              -> Reset slow              -- ARG[5], rst
-              -> Enable slow             -- ARG[6], en
-              -> a                       -- ARG[7]
-              -> a                       -- ARG[8]
-              -> a                       -- ARG[9]
-              -> Signal fast a           -- ARG[10]
-              -> Signal slow (a,a)
+      ddrIn# :: forall a dom domDDR
+              . HasCallStack             -- ARG[0]
+             => NFDataX a                -- ARG[1]
+             => KnownDomain dom          -- ARG[2]
+             => KnownDomain domDDR       -- ARG[3]
+             => DomPeriod ~ 2 * ...      -- ARG[4]
+             => Clock dom                -- ARG[5]
+             -> Reset dom                -- ARG[6]
+             -> Enable dom               -- ARG[7]
+             -> a                        -- ARG[8]
+             -> a                        -- ARG[9]
+             -> a                        -- ARG[10]
+             -> Signal domDDR a          -- ARG[11]
+             -> Signal dom (a,a)
     template: |-
       // ddrIn begin
-      ~SIGD[~GENSYM[data_Pos][1]][9];
-      ~SIGD[~GENSYM[data_Neg][2]][9];
-      ~SIGD[~GENSYM[data_Neg_Latch][3]][9];
-      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[4]~IF~ISSYNC[3]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[5])~FI begin : ~GENSYM[~COMPNAME_ddrIn_pos][6]
-        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[5]~ELSE! ~ARG[5]~FI) begin
-          ~SYM[1] <= ~ARG[8];
-        end else ~IF ~ISACTIVEENABLE[6] ~THEN if (~ARG[6]) ~ELSE ~FI begin
-          ~SYM[1] <= ~ARG[10];
+      ~SIGD[~GENSYM[data_Pos][1]][11];
+      ~SIGD[~GENSYM[data_Neg][2]][11];
+      ~SIGD[~GENSYM[data_Neg_Latch][3]][11];
+      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[5]~IF~ISSYNC[2]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[6])~FI begin : ~GENSYM[~COMPNAME_ddrIn_pos][6]
+        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[6]~ELSE! ~ARG[6]~FI) begin
+          ~SYM[1] <= ~ARG[9];
+        end else ~IF ~ISACTIVEENABLE[7] ~THEN if (~ARG[7]) ~ELSE ~FI begin
+          ~SYM[1] <= ~ARG[11];
         end
       end
-      always @(~IF~ACTIVEEDGE[Rising][2]~THENnegedge~ELSEposedge~FI ~ARG[4]~IF~ISSYNC[3]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[5])~FI begin : ~GENSYM[~COMPNAME_ddrIn_neg][7]
-        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[5]~ELSE! ~ARG[5]~FI) begin
-          ~SYM[2] <= ~ARG[9];
-        end else ~IF ~ISACTIVEENABLE[6] ~THEN if (~ARG[6]) ~ELSE ~FI begin
+      always @(~IF~ACTIVEEDGE[Rising][2]~THENnegedge~ELSEposedge~FI ~ARG[5]~IF~ISSYNC[2]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[6])~FI begin : ~GENSYM[~COMPNAME_ddrIn_neg][7]
+        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[6]~ELSE! ~ARG[6]~FI) begin
           ~SYM[2] <= ~ARG[10];
+        end else ~IF ~ISACTIVEENABLE[7] ~THEN if (~ARG[7]) ~ELSE ~FI begin
+          ~SYM[2] <= ~ARG[11];
         end
       end
-      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[4]~IF~ISSYNC[3]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[5])~FI begin : ~GENSYM[~COMPNAME_ddrIn_neg_latch][8]
-        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[5]~ELSE! ~ARG[5]~FI) begin
-          ~SYM[3] <= ~ARG[7];
-        end else ~IF ~ISACTIVEENABLE[6] ~THEN if (~ARG[6]) ~ELSE ~FI begin
+      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[5]~IF~ISSYNC[2]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[6])~FI begin : ~GENSYM[~COMPNAME_ddrIn_neg_latch][8]
+        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[6]~ELSE! ~ARG[6]~FI) begin
+          ~SYM[3] <= ~ARG[8];
+        end else ~IF ~ISACTIVEENABLE[7] ~THEN if (~ARG[7]) ~ELSE ~FI begin
           ~SYM[3] <= ~SYM[2];
         end
       end
@@ -49,38 +50,40 @@
     kind: Declaration
     outputUsage: Blocking
     type: |-
-      ddrOut# :: ( HasCallStack                -- ARG[0]
-                  , NFDataX a                  -- ARG[1]
-                  , KnownConfi~ fast domf      -- ARG[2]
-                  , KnownConfi~ slow doms      -- ARG[3]
-               => Clock slow                   -- ARG[4]
-               -> Reset slow                   -- ARG[5]
-               -> Enable slow                  -- ARG[6]
-               -> a                            -- ARG[7]
-               -> Signal slow a                -- ARG[8]
-               -> Signal slow a                -- ARG[9]
-               -> Signal fast a
+      ddrOut# :: forall a dom domDDR
+               . HasCallStack                 -- ARG[0]
+              => NFDataX a                    -- ARG[1]
+              => KnownDomain dom              -- ARG[2]
+              => KnownDomain domDDR           -- ARG[3]
+              => DomPeriod ~ 2 * ...          -- ARG[4]
+              => Clock dom                    -- ARG[5]
+              -> Reset dom                    -- ARG[6]
+              -> Enable dom                   -- ARG[7]
+              -> a                            -- ARG[8]
+              -> Signal dom a                 -- ARG[9]
+              -> Signal dom a                 -- ARG[10]
+              -> Signal domDDR a
     template: |-
       // ddrOut begin
-      ~SIGD[~GENSYM[data_Pos][1]][7];
-      ~SIGD[~GENSYM[data_Neg][2]][7];
-      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[4]~IF~ISSYNC[3]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[5])~FI begin : ~GENSYM[~COMPNAME_ddrOut_pos][5]
-        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[5]~ELSE! ~ARG[5]~FI) begin
-          ~SYM[1] <= ~ARG[7];
-        end else ~IF ~ISACTIVEENABLE[6] ~THEN if (~ARG[6]) ~ELSE ~FI begin
+      ~SIGD[~GENSYM[data_Pos][1]][8];
+      ~SIGD[~GENSYM[data_Neg][2]][8];
+      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[5]~IF~ISSYNC[2]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[6])~FI begin : ~GENSYM[~COMPNAME_ddrOut_pos][5]
+        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[6]~ELSE! ~ARG[6]~FI) begin
           ~SYM[1] <= ~ARG[8];
+        end else ~IF ~ISACTIVEENABLE[7] ~THEN if (~ARG[7]) ~ELSE ~FI begin
+          ~SYM[1] <= ~ARG[9];
         end
       end
-      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[4]~IF~ISSYNC[3]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[5])~FI begin : ~GENSYM[~COMPNAME_ddrOut_neg][6]
-        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[5]~ELSE! ~ARG[5]~FI) begin
-          ~SYM[2] <= ~ARG[7];
-        end else ~IF ~ISACTIVEENABLE[6] ~THEN if (~ARG[6]) ~ELSE ~FI begin
-          ~SYM[2] <= ~ARG[9];
+      always @(~IF~ACTIVEEDGE[Rising][2]~THENposedge~ELSEnegedge~FI ~ARG[5]~IF~ISSYNC[2]~THEN)~ELSE or ~IF~ISACTIVEHIGH[2]~THENposedge~ELSEnegedge~FI ~ARG[6])~FI begin : ~GENSYM[~COMPNAME_ddrOut_neg][6]
+        if (~IF~ISACTIVEHIGH[2]~THEN~ARG[6]~ELSE! ~ARG[6]~FI) begin
+          ~SYM[2] <= ~ARG[8];
+        end else ~IF ~ISACTIVEENABLE[7] ~THEN if (~ARG[7]) ~ELSE ~FI begin
+          ~SYM[2] <= ~ARG[10];
         end
       end
 
       always @(*) begin
-        if (~ARG[4]) begin
+        if (~ARG[5]) begin
           ~RESULT = ~IF~ACTIVEEDGE[Rising][2]~THEN~SYM[1]~ELSE~SYM[2]~FI;
         end else begin
           ~RESULT = ~IF~ACTIVEEDGE[Rising][2]~THEN~SYM[2]~ELSE~SYM[1]~FI;

clash-lib/prims/systemverilog/Clash_Intel_DDR.primitives.yaml

@@ -1,42 +1,45 @@
 - BlackBox:
-    name: Clash.Intel.DDR.altddioIn
+    name: Clash.Intel.DDR.altddioIn#
     kind: Declaration
     libraries:
     - altera_mf
     type: |-
-      altddioIn
-        :: ( HasCallStack               -- ARG[0]
-           , KnownConfi~ fast domf      -- ARG[1]
-           , KnownConfi~ slow doms      -- ARG[2]
-           , KnownNat m )               -- ARG[3]
-        => SSymbol deviceFamily         -- ARG[4]
-        -> Clock slow                   -- ARG[5]
-        -> Reset slow                   -- ARG[6]
-        -> Enable slow                  -- ARG[7]
-        -> Signal fast (BitVector m)    -- ARG[8]
-        -> Signal slow (BitVector m,BitVector m)
+      altddioIn#
+        :: forall deviceFamily n dom domDDR
+         . HasCallStack                 -- ARG[0]
+        => KnownDomain dom              -- ARG[1]
+        => KnownDomain domDDR           -- ARG[2]
+        => DomPeriod ~ 2 * ...          -- ARG[3]
+        => DomEdge ~ Rising             -- ARG[4]
+        => KnownNat n                   -- ARG[5]
+        => SSymbol deviceFamily         -- ARG[6]
+        -> Clock dom                    -- ARG[7]
+        -> Reset dom                    -- ARG[8]
+        -> Enable dom                   -- ARG[9]
+        -> Signal domDDR (BitVector n)  -- ARG[10]
+        -> Signal dom (BitVector n, BitVector n)
     template: |-
       // altddioIn begin
-      ~SIGD[~GENSYM[dataout_l][1]][8];
-      ~SIGD[~GENSYM[dataout_h][2]][8];
+      ~SIGD[~GENSYM[dataout_l][1]][10];
+      ~SIGD[~GENSYM[dataout_h][2]][10];
 
       altddio_in
         #(
-          .intended_device_family (~LIT[4]),
+          .intended_device_family (~LIT[6]),
           .invert_input_clocks ("OFF"),
           .lpm_hint ("UNUSED"),
           .lpm_type ("altddio_in"),
           .power_up_high ("OFF"),
-          .width (~SIZE[~TYP[8]])
+          .width (~SIZE[~TYP[10]])
         )
-        ~GENSYM[~COMPNAME_ALTDDIO_IN][7] (~IF ~ISSYNC[2] ~THEN
-          .sclr (~ARG[6]),
+        ~GENSYM[~COMPNAME_ALTDDIO_IN][7] (~IF ~ISSYNC[1] ~THEN
+          .sclr (~ARG[8]),
           .aclr (1'b0),~ELSE
-          .aclr (~ARG[6]),
+          .aclr (~ARG[8]),
           .sclr (1'b0),~FI
-          .datain (~ARG[8]),
-          .inclock (~ARG[5]),
-          .inclocken (~IF ~ISACTIVEENABLE[7] ~THEN~ARG[7]~ELSE1'b1~FI),
+          .datain (~ARG[10]),
+          .inclock (~ARG[7]),
+          .inclocken (~IF ~ISACTIVEENABLE[9] ~THEN~ARG[9]~ELSE1'b1~FI),
           .dataout_h (~SYM[2]),
           .dataout_l (~SYM[1]),
           .aset (1'b0),