No inlining let-bound global vars with clock types

changelog/2024-11-02T17_43_16+08_00_distributive_representative_vec

@@ -0,0 +1 @@
+ADDED: `Distributive` and `Representable` instances for `Vec`

changelog/2024-11-18T14_59_34+01_00_fix2845

@@ -0,0 +1 @@
+FIXED: Clash generates illegal Verilog names [#2845](https://github.com/clash-lang/clash-compiler/issues/2845)

clash-lib/src/Clash/Rewrite/WorkFree.hs

@@ -155,7 +155,18 @@ isWorkFreeClockOrResetOrEnable tcm e =
   if isClockOrReset tcm eTy || isEnable tcm eTy then
     case collectArgs e of
       (Prim p,_) -> Just (primName p == Text.showt 'removedArg)
-      (Var _, []) -> Just True
+      -- Only local variables with a clock type are work-free. When it is a global
+      -- variable, it is probably backed by a clock generator, which is definitely
+      -- not work-free.
+      --
+      -- Inlining let-bindings referencing a global variable with a clock type
+      -- can sometimes lead to the post-normalization flattening stage to generate
+      -- code that violates the invariants of the netlist generation stage.
+      -- Especially when this global binder is defined recursively such as when
+      -- using `tbClockGen`.
+      -- This then ultimately leads to bad verilog names being generated as
+      -- reported in: https://github.com/clash-lang/clash-compiler/issues/2845
+      (Var v, []) -> Just (isLocalId v)
       (Data _, [_dom, Left (stripTicks -> Data _)]) -> Just True -- For Enable True/False
       (Literal _,_) -> Just True
       _ -> Just False

clash-prelude/clash-prelude.cabal

@@ -317,7 +317,8 @@ Library
                       TypeFamilies
                       UndecidableInstances
 
-  Build-depends:      array                     >= 0.5.1.0 && < 0.6,
+  Build-depends:      adjunctions               >= 4.0     && < 5.0,
+                      array                     >= 0.5.1.0 && < 0.6,
                       arrows                    >= 0.4     && < 0.5,
                       base                      >= 4.11    && < 5,
                       binary                    >= 0.8.5   && < 0.11,
@@ -327,6 +328,7 @@ Library
                       data-binary-ieee754       >= 0.4.4   && < 0.6,
                       data-default-class        >= 0.1.2   && < 0.2,
                       deepseq                   >= 1.4.1.0 && < 1.6,
+                      distributive              >= 0.1     && < 1.0,
                       extra                     >= 1.6.17  && < 1.8,
                       ghc-prim                  >= 0.5.1.0 && < 0.12,
                       ghc-typelits-extra        >= 0.4     && < 0.5,

clash-prelude/src/Clash/Sized/Vector.hs

@@ -108,6 +108,8 @@ import Data.Constraint.Nat        (leZero)
 import Data.Data
   (Data (..), Constr, DataType, Fixity (..), Typeable, mkConstr, mkDataType)
 import Data.Either                (isLeft)
+import Data.Distributive
+import Data.Functor.Rep
 #if MIN_VERSION_base(4,18,0)
 import qualified Data.Foldable1   as F1
 #endif
@@ -2776,3 +2778,16 @@ type instance Lens.Index   (Vec n a) = Index n
 type instance Lens.IxValue (Vec n a) = a
 instance KnownNat n => Lens.Ixed (Vec n a) where
   ix i f xs = replace_int xs (fromEnum i) <$> f (index_int xs (fromEnum i))
+
+instance KnownNat n => Distributive (Vec n) where
+    distribute fxs = tabulate $ \i -> fmap (!! i) fxs
+    {-# INLINE distribute #-}
+
+instance KnownNat n => Representable (Vec n) where
+    type Rep (Vec n) = Index n
+
+    tabulate f = map f indicesI
+    {-# INLINE tabulate #-}
+
+    index = (!!)
+    {-# INLINE index #-}

tests/Main.hs

@@ -636,6 +636,7 @@ runClashTest = defaultMain $ clashTestRoot
         , runTest "T2628" def{hdlTargets=[VHDL], buildTargets=BuildSpecific ["TACacheServerStep"], hdlSim=[]}
         , runTest "T2831" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]}
         , runTest "T2839" def{hdlLoad=[],hdlSim=[],hdlTargets=[VHDL]}
+        , runTest "T2845" def{hdlSim=[],hdlTargets=[Verilog]}
         ] <>
         if compiledWith == Cabal then
           -- This tests fails without environment files present, which are only

tests/shouldwork/Issues/T2845.hs

@@ -0,0 +1,13 @@
+module T2845 where
+
+import Clash.Explicit.Prelude
+import Clash.Explicit.Testbench
+
+topEntity ::
+  Signal System (Unsigned 8)
+topEntity = cntr + x
+ where
+  cntr = register clk noReset enableGen 0 0
+  x = register clk noReset enableGen 100 0
+  done = (== 100) <$> cntr
+  clk = tbClockGen $ not <$> done