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

Net names are not escaped properly for vivado tcl #375

Closed
rroohhh opened this issue Apr 30, 2020 · 5 comments
Closed

Net names are not escaped properly for vivado tcl #375

rroohhh opened this issue Apr 30, 2020 · 5 comments

Comments

@rroohhh
Copy link
Contributor

rroohhh commented Apr 30, 2020

It seems like net names are not properly escaped for vivado tcl scripts, atleast $ is causing problems.

This example:

from nmigen import *
from nmigen_boards.zturn_lite_z010 import *

class Test(Elaboratable):
    def elaborate(self, plat):
        m = Module()

        dummy1 = Signal()

        clk = Signal(name="clk$1")
        m.d.comb += dummy1.eq(clk)
        plat.add_clock_constraint(clk, 100_000_000)

        return m


plat = ZTurnLiteZ010Platform()

t = Test()

plat.build(t)

causes vivado to emit the following message:

Parsing XDC File [/data/projects/nmigen_bugs/build/top.xdc]
CRITICAL WARNING: [Common 17-1548] Command failed: can't read "1": no such variable
 [/data/projects/nmigen_bugs/build/top.xdc:2]
Finished Parsing XDC File [/data/projects/nmigen_bugs/build/top.xdc]

dummy1 is merely used to work around / avoid #374.

Vivado fails later due to the design being empty, but I don't think that matters for this bug.

Full build folder: build.zip

@rroohhh
Copy link
Contributor Author

rroohhh commented Apr 30, 2020

Playing around a bit with vivado makes me come to the conclusion, that atleast '"{};$#[]\ are special characters and need to be escaped using \.

Furthermore there seem to be different rules for clock names, alluded to by this error message:

CRITICAL WARNING: [Vivado 12-2270] Clock names may not contain tcl special characters: '"{};$# - Skipping 'clk$1' [/data/projects/nmigen_bugs/build/top.xdc:2]

Which gets emitted when changing the constraint file to this:

create_clock -name clk\$1 -period 10.0 [get_nets \\clk\$1\ ]

@whitequark
Copy link
Member

whitequark commented Apr 30, 2020

I -think- we can get away with using { } around net names and escaping only { and }. (Since we can't guarantee that we'll have a balanced number of those in a signal name, naturally.)

@rroohhh
Copy link
Contributor Author

rroohhh commented Apr 30, 2020

Atleast \ still needs escaping in the case of using { } around net names, atleast if it is the last character of the name.

@whitequark
Copy link
Member

whitequark commented Apr 30, 2020

Yup that's true.

Regarding escaping itself, it'll have to be fully manual. You'd think Jinja2 will make it possible to customize the escaping engine, but nope! Hardcoded to HTML/XML.

@rroohhh
Copy link
Contributor Author

rroohhh commented Apr 30, 2020

Ouch, that is sad.

@whitequark whitequark added this to the 0.3 milestone May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants