Error with Vectors of Clock Reset and Enable

[jacquescomeaux]

I encountered a runtime error in Clash when trying to synthesize VHDL for a larger project and managed to recreate it with this example. Trying to synthesize functionA causes the list-index operator (Prelude.!!) to be called with an index that is too large. The offending !! is in the ArgGen case of Clash.Netlist.BlackBox.Util.renderTag in clash-lib. Meanwhile functionB can be synthesized, and functionC fails with a slightly more informative error.

Is it a normal usage of Clash to pass around Clock, Reset, and Enable signals explicitly like this as the results of functions?

{-# LANGUAGE NoImplicitPrelude     #-}
module Test where

import Clash.Prelude

data CRE = CRE (Clock System) (Reset System) (Enable System)

{-# ANN functionA (defSyn "FunctionA") #-}
{-# ANN functionB (defSyn "FunctionB") #-}
{-# ANN functionC (defSyn "FunctionC") #-}

functionA
    :: Clock System
    -> Reset System
    -> Enable System
    -> Vec 2 (Signal System Bool, CRE)
functionA = \ c r e ->
  let
    term   = CRE c r e
    vec    = term :> term :> Nil
    result = fmap (\a -> (pure True, a)) vec
  in
    result 

functionB
    :: Clock System
    -> Reset System
    -> Enable System
    -> Vec 2 CRE
functionB = \ c r e ->
  let
    term   = CRE c r e
    result = term :> term :> Nil
  in
    result

functionC
    :: Clock System
    -> Reset System
    -> Enable System
    -> Vec 2 CRE
functionC = \ c r e ->
  let
    term   = CRE c r e
    vec    = term :> term :> Nil
    result = fmap id vec
  in
    result

[jacquescomeaux]

Actually, I didn't think to try isolating further by testing out just clocks, resets, or enables by themselves in a vector

[christiaanb]

Ah, interesting. There are a couple of things going on here which result in this bug:

1. Certain higher-order functions over Vec, such as map, have specialized code-paths to turn them into generate-for loops in VHDL, instead of having to having to unroll/inline their recursive definitions:

   clash-compiler/clash-lib/prims/vhdl/Clash_Sized_Vector.json

   Lines 102 to 128 in e6738d1

   	, { "BlackBox" :
   	{ "name" : "Clash.Sized.Vector.map"
   	, "workInfo" : "Never"
   	, "kind" : "Declaration"
   	, "type" : "map :: (a -> b) -> Vec n a -> Vec n b"
   	, "template" :
   	"-- map begin
   	~GENSYM[map][0] : for ~GENSYM[i][1] in ~RESULT'range generate~IF ~VIVADO ~THEN~IF~SIZE[~TYP[1]]~THEN
   	signal ~GENSYM[map_in][2] : ~TYPEL[~TYP[1]];~ELSE ~FI
   	signal ~GENSYM[map_out][3] : ~TYPEL[~TYPO];
   	begin~IF~SIZE[~TYP[1]]~THEN
   	~SYM[2] <= fromSLV(~VAR[vec][1](~SYM[1]));~ELSE ~FI
   	~INST 0
   	~OUTPUT <= ~SYM[3]~ ~TYPEL[~TYPO]~
   	~INPUT <= ~SYM[2]~ ~TYPEL[~TYP[1]]~
   	~INST
   	~RESULT(~SYM[1]) <= ~TOBV[~SYM[3]][~TYPEL[~TYPO]];
   	end generate;~ELSE
   	begin
   	~INST 0
   	~OUTPUT <= ~RESULT(~SYM[1])~ ~TYPEL[~TYPO]~
   	~INPUT <= ~VAR[vec][1](~SYM[1])~ ~TYPEL[~TYP[1]]~
   	~INST
   	end generate;~FI
   	-- map end"
   	}
   	}

2. Clash, in general, translates Haskell product types (like your CRE type) to VHDL records. This mostly works out fine, there is however one exception: certain synthesis tools, and some HDL simulation tools (like verilator), do not like it when the clock (and certain other global control signals) is contained in a record type; they want them to be separate inputs to the entity/module. And Clash actually does some transformations to try to ensure that values of type Clock do not end up in a VHDL record type.

The problem is that the transformations in 2. never took into account the specialized code-paths in 1. We/I will have to think about what the best way forward is.

[christiaanb]

Also, looking at the code for the transformations of 2., I see that it is actually missing a check for "recursive" product types such as Vec.