search

cornell-brg / pymtl

Python-based hardware modeling framework
BSD 3-Clause "New" or "Revised" License
237 stars 82 forks source link

Add a prefix to verilator-xinit comment #174

Closed jsn1993 closed 5 years ago

jsn1993 commented 5 years ago

This is to avoid being recognized as a special verilator comment by verilator. Verilator will sometimes (depending on the version) recognize this // verilator as a verilator special comment, and throw an error when it sees -xinit. 3.876 doesn't have this problem, but 4.008 will. After this modification I got the failing case to work.

jsn1993 commented 5 years ago

https://github.com/grg/verilator/blob/master/src/verilog.l#L757-L784

cbatten commented 5 years ago

Does this mean I can install the latest version of verilator and the latest version of PyMTL on ecelinux for ECE 5745?

jsn1993 commented 5 years ago

Yes it is a comment that PyMTL inserts into the translated/imported verilog. In the above verilator link it shows that the verilator parser does try to parse comments. If a comment line starts with // verilator but the second part is not recognized it throws the error https://github.com/grg/verilator/blob/master/src/verilog.l#L88-L103.

I will need to install it somewhere and make sure it works with all tests.

cbatten commented 5 years ago

I guess my question is does any tool currently try and parse -our- comments? Regardless, what do you think about my suggested syntax?

jsn1993 commented 5 years ago

Hmmmm at least pymtl doesn't reparse the comments.

cbatten commented 5 years ago

OK. Let's go with my syntax if that is okay then ... I think it makes it more clear that these are PyMTL comments that at least could be parsed in the future if we needed to.

cbatten commented 5 years ago

Can you test and merge in today so we can use the most recent version of verilator and PyMTL in ECe 5745?