Different ILAs are linked to the same IP backbox

[kleinreact]

The following example instantiates two ILA blackboxes, which get mapped to the same IP instance in Vivado. Thus, the first ILA silently disappears. Clash should either produce different names for the generated IP or complain about such a kind of instantiation.

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))

Generates topEntity2.v containing

...
topEntity_ila_E981CE816A1232C3 topEntity_probe___VOID_TDECL_NOOP___0
  (.clk (c$pTS0), .probe0 (a));
...

and topEntity2.v containing

...
topEntity_ila_E981CE816A1232C3 topEntity_probe___VOID_TDECL_NOOP__
  (.clk (c$pTS0), .probe0 (b));
...

but only a single topEntity_ila_E981CE816A1232C3.clash.tcl

[christiaanb]

It’s on the Blackbox/Primitive writer that Clash does not deduplicate the so-called “extra” files. Their content should be different if you want two TCL files.

It’s highly unusual that two components that are exactly the same should each get a separate file/name.

[kleinreact]

It’s highly unusual that two components that are exactly the same should each get a separate file/name.

Yes, I guess that ILAs and VIOs will be the only exception to that.

[christiaanb]

Perhaps the ILA config can come with a name that you add as a comment in the TCL file. That should make the TCL files different enough for deduplication to not happen.

[christiaanb]

On a separate note, looking at the generated Verilog, it seems that

clash-compiler/clash-cores/src/Clash/Cores/Xilinx/Ila/Internal.hs

Line 278 in 5e01ae2

getIlaName (Just "__VOID_TDECL_NOOP__") = getIlaName Nothing

is not working.

[DigitalBrains1]

It is truly strange that you can't instantiate the same ILA twice, but I suppose we can add something to the Clash<->Tcl API that handles such a case specifically. Do you have documentation that says this is not possible? It is possible for any normal IP core, so you'd think they'd document when that suddenly deviates.

Note that the instance names are unique (topEntity_probe___VOID_TDECL_NOOP___0 and topEntity_probe___VOID_TDECL_NOOP__), it is only the name of the IP core that is the same.

[edit]
The solutions offered by Christiaan above would also work fine, but we can also solve it in a more principled way with the Clash<->Tcl API. We're not actually interested in deduplicating the .clash.tcl file, we're interested in deduplicating the IP cores.
[/edit]

[kleinreact]

The documentation states:

Component Name – Use this text field to provide a unique module name for the ILA core.

It reads like each ILA instance must be associated a unique name. Hence, we probably have to instantiate a different ipName for both of the ILA instances here.

This is the generated topEntity_ila_E981CE816A1232C3.clash.tcl for reference:

namespace eval $tclIface {
  variable api 1
  variable scriptPurpose createIp
  variable ipName {topEntity_ila_E981CE816A1232C3}
  proc createIp {ipName0 args} {
    create_ip \
      -name ila \
      -vendor xilinx.com \
      -library ip \
      -version 6.2 \
      -module_name $ipName0 \
      {*}$args

    set_property \
      -dict [list \
                  CONFIG.C_NUM_OF_PROBES 1 \
                  CONFIG.C_INPUT_PIPE_STAGES 0 \
                  CONFIG.C_DATA_DEPTH 4096 \
                  CONFIG.ALL_PROBE_SAME_MU true \
                  CONFIG.C_EN_STRG_QUAL 1 \
                  CONFIG.C_TRIGIN_EN false \
                  CONFIG.C_ADV_TRIGGER false \
                  CONFIG.ALL_PROBE_SAME_MU_CNT 2 \
                  CONFIG.C_PROBE0_WIDTH 1 \
                  CONFIG.C_PROBE0_TYPE 0 \
                  CONFIG.C_PROBE0_MU_CNT 2 \
            ] [get_ips $ipName0]
    return
  }
}

As there have to be unique probe names as well, they should be taken into account via IlaConfig.

[DigitalBrains1]

Ah wait, my idea doesn't work. I wanted to modify clashConnector.tcl to instantiate multiple modules when the API specifically calls out this situation (say variable uniqueModules true), each with a unique name. But we don't have a mapping available from modules to instantiations that would enable this. I think Christiaans solution of adding a unique string to the .clash.tcl file will have to be the proper solution.

It reads like each ILA instance must be associated a unique name.

I doubt it should be read like that. It literally says "for the ILA core". These things are called IP cores; you normally create one IP core which you can happily instantiate multiple times. One instance is not called a "core". It's "an instance of a core". We instantiate multiple instances all the time for everything else that generates a .clash.tcl file.

[kleinreact]

This issue can be closed, as ILA's being linked to the same blackbox IP is the desired behavior and two instances of this IP are correctly generated.

The origin for this issue roots in the incorrect naming of the ILA's in an automated setup, which caused multiple ILAs to be connected to the same external sink appearing like there would be only one ILA in the first place (see #2655 for a more detailed background on this).

[DigitalBrains1]

the incorrect naming of the ILA's in an automated setup

The naming of the ILAs was not the problem, it was that the script of the automated setup in question shortened the name and exactly threw out the bit that was unique, instead opting for the part of the name that was the least unique. I think that is important to point out here: the ILA's all had unique names, it was a simple confusion on the part of the author of that script.

For this reason, #2655 is not really related. If two ILA's were to have been instantiated with setName @"foo" and setName @"bar", the script would still have given them the same name if they lived in the same instance at the top level.