Skip to content

Fix higher order blackboxes not propagating usage metadata#3148

Open
rowanG077 wants to merge 1 commit intomasterfrom
3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments
Open

Fix higher order blackboxes not propagating usage metadata#3148
rowanG077 wants to merge 1 commit intomasterfrom
3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments

Conversation

@rowanG077
Copy link
Copy Markdown
Member

@rowanG077 rowanG077 commented Feb 10, 2026 •
edited
Loading

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

@rowanG077 rowanG077 linked an issue Feb 10, 2026 that may be closed by this pull request
Verilog blackbox for iterateI uses wire for sequential assignments #3147
Open
@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch from 755916f to c52cf26 Compare February 10, 2026 18:30
@rowanG077 rowanG077 changed the title FIXED: HO blackboxes not propagating usage metadata Fix HO blackboxes not propagating usage metadata Feb 10, 2026
@rowanG077 rowanG077 requested a review from lmbollen February 10, 2026 18:33
@rowanG077
Copy link
Copy Markdown
Member Author

rowanG077 commented Feb 10, 2026 •
edited
Loading

Sad to say neither verilator or yosys catch this as a warning. And just text based checks really sucks because on my machine this text based check passes but I guess with different GHC/Clash combinations the exact ordering can be different.

@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch 2 times, most recently from 10982ba to 53fee98 Compare February 11, 2026 10:19
DigitalBrains1
DigitalBrains1 reviewed Feb 11, 2026
View reviewed changes
@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch from 53fee98 to 507b203 Compare February 11, 2026 13:59
@rowanG077 rowanG077 changed the title Fix HO blackboxes not propagating usage metadata Fix higher order blackboxes not propagating usage metadata Feb 11, 2026
@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch from 507b203 to 661931c Compare February 11, 2026 14:01
@rowanG077
Copy link
Copy Markdown
Member Author

rowanG077 commented Feb 11, 2026 •
edited
Loading

The given reproducer in the issue actually wasn't a reproducer, but with explicit clocks it does reproduce.

For the verification all sequential assignments to identifiers are gathered and it's checked the identifiers are declared as reg.

martijnbastiaan
martijnbastiaan approved these changes Feb 11, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure on the fix, but the tests look like they actually test something sensible to ✔️

@rowanG077
Copy link
Copy Markdown
Member Author

rowanG077 commented Feb 11, 2026 •
edited
Loading

Let's wait for the grandmaster @christiaanb to see if he thinks this is the proper fix.

@rowanG077 rowanG077 requested review from christiaanb and removed request for lmbollen February 11, 2026 17:33
@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch 3 times, most recently from 91449e2 to 31104d3 Compare April 1, 2026 10:03
christiaanb
christiaanb reviewed Apr 1, 2026
View reviewed changes
clash-lib/src/Clash/Primitives/DSL.hs
-- Pick the fSubPos-th HO instantiation (if present) and reuse its usage.
case IntMap.lookup fPos (bbFunctions bbCtx) >>= listToMaybe . drop fSubPos of
Just (_, usage, _, _, _, _) -> declareUseOnce usage resName
Nothing -> pure ()
Copy link
Copy Markdown
Member

@christiaanb christiaanb Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what we want? Should we be allowed to instantiate a function argument more often that its function plurality? Shouldn't we error out if that happens? @martijnbastiaan

@rowanG077 rowanG077 force-pushed the 3147-verilog-blackbox-for-iteratei-uses-wire-for-sequential-assignments branch from 31104d3 to d2b317a Compare April 1, 2026 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christiaanb christiaanb christiaanb left review comments

@DigitalBrains1 DigitalBrains1 DigitalBrains1 left review comments

@martijnbastiaan martijnbastiaan martijnbastiaan approved these changes

@leonschoorl leonschoorl Awaiting requested review from leonschoorl

Assignees

No one assigned

Labels

backport 1.8

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Verilog blackbox for iterateI uses wire for sequential assignments

4 participants

@rowanG077 @christiaanb @DigitalBrains1 @martijnbastiaan