clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.4k stars 147 forks source link

`__VOID_TDECL_NOOP__` is not correctly matched #2654

Open kleinreact opened 5 months ago

kleinreact commented 5 months ago

As discovered during the discussion of #2649 by @christiaanb, the following pattern match does not seem to work correctly https://github.com/clash-lang/clash-compiler/blob/5e01ae2aec13f7b91cc8a6f5475feb4f24bf50b5/clash-cores/src/Clash/Cores/Xilinx/Ila/Internal.hs#L278 as the following example

module Test where

import Clash.Explicit.Prelude
import Clash.Cores.Xilinx.Ila

topEntity :: Clock System -> Signal System ()
topEntity clk =
    hwSeqX (ilaInst (ilaConfig ("a" :> Nil)))
  $ hwSeqX (ilaInst (ilaConfig ("b" :> Nil)))
    pure ()
 where
  ilaInst :: IlaConfig 1 -> Signal System ()
  ilaInst cfg = ila cfg clk (pure 0 :: Signal System (Unsigned 1))

still produces

...
topEntity_ila_E981CE816A1232C3 topEntity_probe___VOID_TDECL_NOOP___0
  (.clk (c$pTS0), .probe0 (a));
...
topEntity_ila_E981CE816A1232C3 topEntity_probe___VOID_TDECL_NOOP__
  (.clk (c$pTS0), .probe0 (b));
...
DigitalBrains1 commented 5 months ago

That's because the name is Just "topEntity_probe___VOID_TDECL_NOOP__", not just the suffix.

DigitalBrains1 commented 5 months ago

This is related to my comment in #2655: as soon as Clash needs to deduplicate, as here, it will prefix the name with a prefix based on name of the entity containing the ILA. At that point the pattern no longer matches. The pattern was probably only tested with a single ILA.

If I only leave one ILA in your code above, I get this bbCtxName:

Just "__VOID_TDECL_NOOP__"

but with the original code with two ILAs they are:

Just "topEntity_ilaInst___VOID_TDECL_NOOP__"
Just "topEntity_ilaInst___VOID_TDECL_NOOP__"

Note they haven't been deduplicated yet.

[edit] Huh... the prefix isn't the same as yours. I'm going to guess we are using different versions of GHC and the Core contains subtly different names. I used GHC 9.6.3 on latest Clash master. [/edit]