clash-lang / clash-compiler

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

ILA uncomfortable to use when using many probes #2717

Open martijnbastiaan opened 4 months ago

martijnbastiaan commented 4 months ago

I've got this beauty:

  debugIla :: Signal freeclk ()
  debugIla = ila
    ((ilaConfig $
         "ila_probe_maybeAlignedRxData0"
      :> "ila_probe_gtwiz_userdata_tx_in"
      :> "ila_probe_reset_rx_done"
      :> "ila_probe_reset_tx_done"
      :> "ila_probe_rst_all_in"
      :> "ila_probe_validAlign"
      :> "ila_probe_alignRst"
      :> "ila_probe_prbsErrors"
      :> "ila_probe_alignBits"
      :> "ila_probe_metaBits"
      :> "ila_probe_alignedAlignBits"
      :> "ila_probe_alignedMetaBits"
      :> "ila_probe_linkUp"
      :> "ila_probe_stats_txRetries"
      :> "ila_probe_stats_rxRetries"
      :> "ila_probe_stats_rxFullRetries"
      :> "ila_probe_stats_failAfterUps"
      :> "capture"
      :> "trigger"
      :> Nil) { advancedTriggers = True, stages = 1, depth = D2048 })
    freeclk
    (xpmCdcArraySingle rx_clk freeclk maybeAlignedRxData0)
    (xpmCdcArraySingle tx_clk freeclk gtwiz_userdata_tx_in)
    (xpmCdcArraySingle rx_clk freeclk reset_rx_done)
    (xpmCdcArraySingle tx_clk freeclk reset_tx_done)
    (unsafeToActiveHigh rst_all_in)
    (xpmCdcArraySingle rx_clk freeclk validAlign)
    (xpmCdcArraySingle rx_clk freeclk (unsafeToActiveHigh alignRst))
    (xpmCdcMaybeLossy  rx_clk freeclk (Just <$> prbsErrors))
    (xpmCdcMaybeLossy  rx_clk freeclk (Just <$> alignBits))
    (xpmCdcMaybeLossy  rx_clk freeclk (Just <$> metaBits))
    (xpmCdcMaybeLossy  rx_clk freeclk maybeAlignedAlignBits)
    (xpmCdcMaybeLossy  rx_clk freeclk maybeAlignedMetaBits)
    (xpmCdcSingle rx_clk freeclk result.linkUp)
    ((.txRetries) <$> stats)
    ((.rxRetries) <$> stats)
    ((.rxFullRetries) <$> stats)
    ((.failAfterUps) <$> stats)
    (pure True :: Signal freeclk Bool) -- capture
    (    xpmCdcSingle tx_clk freeclk userDataTx
    .&&. xpmCdcSingle rx_clk freeclk userDataRx
    .&&. unsafeToActiveLow rst_all_in
    ) -- trigger

I feel this it is now very error prone to add a probe, as I cannot easily see which probe name matches to which signal. We should use an inst like invocation:

ila
  ilaConfig{advancedTriggers = True, stages = 1, depth = D2048}
  freeclk
  (Port @"ila_probe_maybeAlignedRxData0" (xpmCdcArraySingle rx_clk freeclk maybeAlignedRxData0))
  (Port @"foo" ...)
  (Port @"bar" ...)
basile-henry commented 4 months ago

(Port @"foo" ...)

And then you can go one step further and use a typeclass to name ports that contain multiple signals. This way you can do Port @"some_dataflow" x and get names for "some_dataflow_valid", "some_dataflow_ready" and "some_dataflow_dat" (for example). This is also useful for top level/synthesis units. :smile: