Generated VHDL - Signals assigned in reset not in sensitivity list

[mikechong]

Hi, I've noticed that in quite a number of the process blocks in the generated VHDL are missing some signals from the sensitivity list. For example:

  -- regEn begin
  topentity_parser_regen_0 : process(system1000,system1000_rstn,x_6_app_arg,result_3)
  begin
    if system1000_rstn = '0' then
      x_6 <= (packetheader_sel0 => x_6_app_arg,packetheader_sel1 => unsigned(x_6_app_arg_0),packetheader_sel2 => unsigned(x_6_app_arg_1));
    elsif rising_edge(system1000) then
      if result_3 then
        x_6 <= headera;
      end if;
    end if;
  end process;
  -- regEn end

In this example, x_6_app_arg_0 and x_6_app_arg_1 are not included in the sensitivity list. So far I've only seen this problem in the reset assignments.

[leonschoorl]

It was my understanding that sensitivity list contains signals which cause the process to "run". In this case, a register with an asynchronous reset, it should only need the clock and reset in the sensitivity list.

[leonschoorl]

Sorry, I was mistaken.

When this register is held in reset it will update its output(x_6) when parts of it's reset value(x_6_app_arg) change, but not when other parts (x_6_app_arg_0, .._1) change. This can't be the intended meaning and will probably cause a synthesis tool to generate warning and/or infer uninteded latches.

But it seems you're using regEn, and it's reset value is not a Signal a but just an a. So the reset value can't change over time, it is static. And it shouldn't be an issue, because the reset value will never change.

I tried to recreate this VHDL output, but I couldn't. Could you share some code which generates processes like this?

[christiaanb]

This is indeed a bug, looking at:https://github.com/clash-lang/clash-compiler/blob/33b6e62e9304e3bef8cb48ea7374847b3ca8d8f5/clash-vhdl/primitives/CLaSH.Signal.Internal.json#L31

we see a ~VARS[1]~VARS[2] in the sensitivity list, which means, all the variables used in the second argument and the third arguments. Where the second argument is the reset value, and the third argument is the enable.

However, it seems that not all variables of the reset value were picked up. Looking at: https://github.com/clash-lang/clash-compiler/blob/33b6e62e9304e3bef8cb48ea7374847b3ca8d8f5/clash-lib/src/CLaSH/Netlist/BlackBox/Util.hs#L369-L381

this can happen when the reset value is the application of a primitive.

@mikechong Can you provide the Haskell /Clash code that generates this incorrect VHDL.?

[mikechong]

Hi Leon and Christiaan,

Thanks for the quick response. The following is a cut down version of the code where we were seeing this issue, but still reproduces the issue:

{-# LANGUAGE RecordWildCards #-}

module TopEntity where

import CLaSH.Prelude

data MessageHeader = MessageHeader {
    field1                                   :: Unsigned 8,
    field2                                   :: Bool
} deriving (Show, Eq)

instance BitPack MessageHeader where
    type BitSize MessageHeader = 9
    pack MessageHeader{..} =
        pack(field2) ++#
        pack(field1)

    unpack x = MessageHeader
        (unpack (slice (SNat @ 7) (SNat @ 0) x) :: Unsigned 8)
        (unpack (slice (SNat @ 8) (SNat @ 8) x) :: Bool)

instance Default MessageHeader where
    def = unpack 0

topEntity :: Signal (BitVector 32) -> Signal Bool -> Signal MessageHeader
topEntity dataInA enA = headerA'
    where
    parseHeader :: BitVector 32 -> MessageHeader
    parseHeader rawData = unpack $ slice d31 d23 rawData

    headerA  = parseHeader <$> dataInA
    headerA' = regEn def enA headerA

and the following gets generated:

  -- regEn begin
  topentity_topentity_0_regen : process(system1000,system1000_rstn,ena)
  begin
    if system1000_rstn = '0' then
      result <= (messageheader_sel0 => unsigned(result_app_arg),messageheader_sel1 => result_app_arg_0 = (std_logic_vector'("1")));
    elsif rising_edge(system1000) then
      if ena then
        result <= app_arg_0;
      end if;
    end if;
  end process;
  -- regEn end

Seems to be related to using Default as the initial state of the register.

[leonschoorl]

The new clash-0.99 (from git) does not have this problem anymore (at least in this specific instance). It evaluates the reset value to a constant which is directly inlined in the register, which I think should always be possible.

  -- register begin
  senslist_topentity_register : block
    signal clk_0 : std_logic;
    signal ce : boolean;
  begin
    (clk_0,ce) <= \#app_arg\;
    senslist_topentity_reg : process(clk_0,rst)
    begin
      if rst = '1' then
        result <= (messageheader_sel0 => to_unsigned(0,8),messageheader_sel1 => false);
      elsif rising_edge(clk_0) then
        if ce then
          result <= \#app_arg_0\
          -- pragma translate_off
          after 1 ps
          -- pragma translate_on
          ;
        end if;
      end if;
    end process;
  end block;
  -- register end