Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vivado does not accept clock names with $ in Verilog #2242

Open
4 tasks
lmbollen opened this issue Jun 16, 2022 · 4 comments · May be fixed by #2492
Open
4 tasks

Vivado does not accept clock names with $ in Verilog #2242

lmbollen opened this issue Jun 16, 2022 · 4 comments · May be fixed by #2492

Comments

@lmbollen
Copy link
Contributor

lmbollen commented Jun 16, 2022

Even though $ is an accepted character in Verilog's basic identifiers. Vivado does not accept it in clock names.
A constraint file with
create_clock -name {c$arg} -period 5.000 -waveform {0.000 2.500} [get_ports {c$arg}]
results in
[Vivado 12-2270] Clock names may not contain tcl special characters: '"{};$# - Skipping 'c$arg' [/filepath]

Possible fixes

  • Adjust -fclash-hdlsyn Vivado to not generate $ in basic identifiers.
  • Generate more basic identifiers by default (excluding tcl special characters?).
  • Never generate tcl special characters in clock names.
  • Change the c$ prefix.
@DigitalBrains1
Copy link
Member

Note that changing the c$ prefix doesn't help; if you use an implicit clock, the argument is named like $d(%,%) which maps to the basic Verilog identifier _$d_0 exhibiting the same issue.

@DigitalBrains1
Copy link
Member

Here's a small reproducer

module ImplClock whereimport Clash.PreludetopEntity ::
  HiddenClock System =>
  Signal System (Unsigned 8) ->
  Signal System (Unsigned 8)
topEntity = dflipflop
{-# NOINLINE topEntity #-}

Note that this issue means f6a25e0 was an incomplete fix, as it only fixed VHDL, but not Verilog.

@alex-mckenna
Copy link
Contributor

We're going to work on fixing this at ZuriHac

NadiaYvette added a commit to NadiaYvette/clash-compiler that referenced this issue Jun 10, 2023
@alex-mckenna alex-mckenna linked a pull request Jun 10, 2023 that will close this issue
@alex-mckenna
Copy link
Contributor

Changing the ID generation is indeed a little hairy here. The identifiers which cause problems ultimately come from unsafeFromCoreId, meaning we don't do the sanitising that we get from makeBasic etc. We probably don't want to just change this function to strip out characters like $ because some names might be specified by users directly in things like synthesize annotations.

Perhaps a solution could be to add a flag to actually do this mangling, i.e. -fclash-sanitise-netlist-ids, which does this stripping of potentially troublesome characters with the caveat that user-specified names that end up going through unsafeFromCoreId may no longer stay as exactly what the user specified.

NadiaYvette added a commit to NadiaYvette/clash-compiler that referenced this issue Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants