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

Prune Yosys-internal attributes in emitted Verilog #220

Closed
whitequark opened this issue Sep 21, 2019 · 15 comments
Closed

Prune Yosys-internal attributes in emitted Verilog #220

whitequark opened this issue Sep 21, 2019 · 15 comments

Comments

@whitequark
Copy link
Contributor

Yosys-internal attributes like init can confuse vendor tools, with potentially severe consequences. We should remove them.

Ideally we would map src to something sensible, but I'm not sure if this is supported by any vendor tools.

@Ravenslofty
Copy link
Contributor

Ravenslofty commented Sep 21, 2019

Can we add toolchain:quartus too? It accepts but warns on every one it doesn't recognise.

@daveshah1
Copy link

Would need some work in the Verilog backend, but some cases of src might be mappable to the `line preprocessor command

@whitequark
Copy link
Contributor Author

whitequark commented Sep 21, 2019

@ZirconiumX Done

@daveshah1 Is there a `file?

@whitequark
Copy link
Contributor Author

whitequark commented Sep 21, 2019

Oh nevermind, it's `line <line> "<filename>". That would be really nice to have.

@daveshah1
Copy link

`line can take a filename

Screenshot_20190921_161742

@whitequark
Copy link
Contributor Author

Ah crap, this is really bad--it's not possible to remove init with attrmap since it would also erase it from the actual register in the Verilog output.

@whitequark
Copy link
Contributor Author

YosysHQ/yosys#1393

@whitequark
Copy link
Contributor Author

In ISE, it is not possible to use user-defined attributes for anything. ISE also loudly complains about seeing any nmigen.* attributes, as well as src attributes. So all of these should be stripped (perhaps leaving around a top.debug.v file with the attributes present).

In Vivado, it is possible to use user-defined attributes, and indeed we do use it. Vivado does not complain about user-defined attributes, so there is no need to strip any of them.

@whitequark
Copy link
Contributor Author

In Diamond, it appears to not be possible to use user-defined attributes for anything. (ncd_attr/ngd_attr can only manipulate built-in ones, and only post-synthesis anyway.) Moreover, edif2ngd complains about all of our attributes (probably anything except syn_*), so these should be stripped.

@Ravenslofty
Copy link
Contributor

Warning (10335): Unrecognized synthesis attribute "nmigen.hierarchy" at top.v(4) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 4
Warning (10335): Unrecognized synthesis attribute "generator" at top.v(5) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 5
Warning (10335): Unrecognized synthesis attribute "src" at top.v(7) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 7
Warning (10335): Unrecognized synthesis attribute "src" at top.v(9) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 9
Warning (10335): Unrecognized synthesis attribute "src" at top.v(11) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 11
Warning (10335): Unrecognized synthesis attribute "src" at top.v(13) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 13
Warning (10335): Unrecognized synthesis attribute "src" at top.v(15) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 15
Warning (10335): Unrecognized synthesis attribute "src" at top.v(17) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 17
Warning (10335): Unrecognized synthesis attribute "src" at top.v(19) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 19
Warning (10335): Unrecognized synthesis attribute "src" at top.v(21) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 21
Warning (10335): Unrecognized synthesis attribute "src" at top.v(23) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 23
Warning (10335): Unrecognized synthesis attribute "src" at top.v(25) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 25
Warning (10335): Unrecognized synthesis attribute "src" at top.v(27) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 27
Warning (10335): Unrecognized synthesis attribute "src" at top.v(29) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 29
Warning (10335): Unrecognized synthesis attribute "src" at top.v(31) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 31
Warning (10335): Unrecognized synthesis attribute "src" at top.v(33) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 33
Warning (10335): Unrecognized synthesis attribute "src" at top.v(35) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 35
Warning (10335): Unrecognized synthesis attribute "src" at top.v(37) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 37
Warning (10335): Unrecognized synthesis attribute "src" at top.v(39) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 39
Warning (10335): Unrecognized synthesis attribute "src" at top.v(41) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 41
Warning (10335): Unrecognized synthesis attribute "src" at top.v(43) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 43
Warning (10335): Unrecognized synthesis attribute "src" at top.v(45) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 45
Warning (10335): Unrecognized synthesis attribute "src" at top.v(47) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 47
Warning (10335): Unrecognized synthesis attribute "src" at top.v(50) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 50
Warning (10335): Unrecognized synthesis attribute "src" at top.v(52) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 52
Warning (10335): Unrecognized synthesis attribute "src" at top.v(55) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 55
Warning (10335): Unrecognized synthesis attribute "init" at top.v(57) File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 57

This is an example of the kind of spam that Quartus spits out when I try building any nMigen code. It also spits out warnings such as

Warning (10270): Verilog HDL Case Statement warning at top.v(1878): incomplete case statement has no default case item File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 1878
Warning (10935): Verilog HDL Casex/Casez warning at top.v(1907): casex/casez item expression overlaps with a previous casex/casez item expression File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 1907

But I think those are/should be separate issues.

@whitequark
Copy link
Contributor Author

In iCECube2 with LSE, all of our custom attributes appear to be ignored, or at least there are no complaints. In iCECube2 with Synplify, Synplify complains about all of our custom attributes. So I think they should be stripped in both cases.

@whitequark
Copy link
Contributor Author

Quartus, as @ZirconiumX shows above, complains about all of our custom attributes. So they should again be stripped.

Warning (10270): Verilog HDL Case Statement warning at top.v(1878): incomplete case statement has no default case item File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 1878
Warning (10935): Verilog HDL Casex/Casez warning at top.v(1907): casex/casez item expression overlaps with a previous casex/casez item expression File: //wsl$/Ubuntu-18.04/home/lofty/de_10/build/top.v Line: 1907

That's not actually an issue, just Quartus being obnoxious.

whitequark added a commit that referenced this issue Sep 24, 2019
Although useful for debugging, most external tools often complain
about such attributes (with notable exception of Vivado). As such,
it is better to emit Verilog with these attributes into a separate
file such as `design.debug.v` and only emit the attributes that were
explicitly placed by the user to `design.v`.

This still leaves the (*init*) attribute. See #220 for details.
whitequark added a commit that referenced this issue Sep 24, 2019
Although useful for debugging, most external tools often complain
about such attributes (with notable exception of Vivado). As such,
it is better to emit Verilog with these attributes into a separate
file such as `design.debug.v` and only emit the attributes that were
explicitly placed by the user to `design.v`.

This still leaves the (*init*) attribute. See #220 for details.
@whitequark
Copy link
Contributor Author

After 53bb430 I strip all internal attributes except init for top.v, and leave them all in top.debug.v, consistently for all platforms. init still has to be handled upstream. src could or could not be handled upstream as well, one day.

@whitequark whitequark removed this from the 0.1 milestone Oct 12, 2019
@whitequark
Copy link
Contributor Author

I've decided against stripping (*init*) attributes in nMigen itself using regular expressions, and rather waiting for a proper implementation to land in Yosys. Therefore, nothing is to be done here for 0.1.

@whitequark
Copy link
Contributor Author

Fixed in Yosys master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants