VIO and ILA: explicitly check setName#2662
Conversation
If `Clash.Magic.setName` was used, use that name for the instance. Otherwise, use a fixed default name. Fixes #2654
|
Interesting! Didn't know this was a thing. Should we add a netlist output regression test? |
| ilaBBF :: HasCallStack => BlackBoxFunction | ||
| ilaBBF _isD _primName args _resTys = Lens.view tcCache >>= go | ||
| ilaBBF _isD _primName args _resTys = do | ||
| instName <- fromMaybe "ila_inst" <$> Lens.view setName |
There was a problem hiding this comment.
| instName <- fromMaybe "ila_inst" <$> Lens.view setName | |
| instName <- fromMaybe "ila_inst" <$> traverse affixName (Lens.view setName) |
The above really is just a suggestion. Normally we add suffixes and prefixes to setName. I know this is not what we want for ILAs, but now the ILA is the odd one out with regards to setName.
There was a problem hiding this comment.
No, I agree, this was a simple oversight. This is not the place to strip affixes; if we want to do that, we should do it elsewhere.
What exactly would it test; that it doesn't regress to issue #2654? So, in essence that the netlist output should not contain My tired brain won't come up with an analysis how useful this is, so I'm just going to go with your opinion. If you want it, I'll write it. |
We already have HDL tests that check whether something is not in the generated HDL: So I guess it could work like that? |
4 participants
If
Clash.Magic.setNamewas used, use that name for the instance. Otherwise, use a fixed default name.Fixes #2654
Still TODO:
Write a changelog entry (see changelog/README.md)