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

Support +systemverilogext+ #532

Closed
veripoolbot opened this issue Jul 19, 2012 · 9 comments
Closed

Support +systemverilogext+ #532

veripoolbot opened this issue Jul 19, 2012 · 9 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Jul 19, 2012


Author Name: Jeremy Bennett (@jeremybennett)
Original Redmine Issue: 532 from https://www.veripool.org
Original Date: 2012-07-19
Original Assignee: Jeremy Bennett (@jeremybennett)


VCS provides an option, @+systemverilogext+@, to specify that a particular file extension is associated with SystemVerilog source code. This is particularly useful when mixing new SystemVerilog code with legacy Verilog code, which uses variables that are keywords in SystemVerilog.

This should be relatively straightforward to implement, since Verilator already enforces language type by inserting @begin_keywords@ into the lexer's token stream. This just has to be made dependent on any extensions specified.

For completeness, we really ought to add

  • @+verilogext+@
  • @+1364-1995ext+@
  • @+1364-2001ext+@
  • @+1364-2001ext+@
  • @+1364-2005ext+@
  • @+1800-2005ext+@
  • @+VAMS-2.3ext+@
  • @+1800+VAMSext+@

I'm happy to implement this, as soon as I have finished my current work on COMBDLY.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Jul 19, 2012


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-07-19T16:10:45Z


Makes sense. FYI VCS supports

+systemverilogext+
+verilog1995ext+
+verilog2001ext+

So I'd suggest that set.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 7, 2012


Original Redmine Comment
Author Name: Jeremy Bennett (@jeremybennett)
Original Date: 2012-11-07T19:52:48Z


I have created a patch for this. It adds the following:

  • +1364-1995ext+
  • +1364-2001ext+
  • +1364-2001ext+
  • +1364-2005ext+
  • +1800-2005ext+
  • +systemverilogext+
  • +verilog1995ext+
  • +verilog2001ext+

Please pull the patch from branch langext at https://github.com/jeremybennett/verilator.

I have added a new option --default-language as a synonym for --language, which I suggest should now be deprecated. I have corrected the list of permitted languages that may be passed to --language as an option.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 8, 2012


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-11-08T03:46:59Z


Does another simulator support -default-language, or is the rename just for making it more obvious? I use --language in a bunch of other tools, so while perhaps not the best name want it to remain. We could retain both I suppose (not mark --language deprecated.)

Also are you using the language() in the FileLine? It might be the right spot (you found my comment to that :) but it is an expensive place to store it if it will not be used since there's one structure for each line of source (potentially millions.) If it is going to now be there, then you need to update the constructor, copy and compare methods etc to also look at that new member.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 12, 2012


Original Redmine Comment
Author Name: Jeremy Bennett (@jeremybennett)
Original Date: 2012-11-12T16:54:22Z


I only changed to --default-language, because I thought it was less confusing. I've reworded, so it is just an alternative to --language.

Good point about the space implications. Since language is associated with a filename, I've added it as a new table in @FileLineSingleton@. The constructor sets an appropriate default. This avoids the space issue. There are no regression failures.

Please pull the patch from branch langext at https://github.com/jeremybennett/verilator.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 12, 2012


Original Redmine Comment
Author Name: Jeremy Bennett (@jeremybennett)
Original Date: 2012-11-12T19:31:53Z


Hold fire on this patch. I realize it doesn't quite work properly. In @parseFile() am@ associating the language with the initial @fileline@ (where the file is being opened from), not the @modfilename@ that is being opened.

New patch shortly...

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 12, 2012


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-11-12T19:35:01Z


Note also at present it's legal to change the language between modules within one filename.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 13, 2012


Original Redmine Comment
Author Name: Jeremy Bennett (@jeremybennett)
Original Date: 2012-11-13T12:45:22Z


Fault corrected. All regression tests pass.

The language from which tokens are recognized can be changed within a filename, by using begin_keywords. This still works, since it uses the scanner (verilog.l) in the same way as before. However the semantics of the language being recognized are not changed by begin_keywords (IEEE 1800-2009 section 22.14 "...The directives do not affect the semantics, tokens, and other aspects of the SystemVerilog language...".

The --language/--default-language and the various +ext+ options define the language semantics being followed, and this is stored on a per-filename basis. I believe this is the same for other simulators. If a source file is used twice, with an intervening +ext+ option, the semantics used would change, and potentially cause a problem. The only way to avoid this is to put the language in the FileLine, rather than FileLineSingleton, with the consequent increase in AST size. However I believe this mode of operation is so unlikely as to be not worth worrying about.

Strictly speaking `begin_keywords is a SystemVerilog construct, but it is helpful to allow it in Verilog source code as well.

Please pull the corrected patch from branch langext at https://github.com/jeremybennett/verilator.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 14, 2012


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-11-14T01:14:50Z


Pushed to git towards 3.843.

Only minor changes; most notably put the code in V3LangCode.cpp back into V3Options.cpp because each additional object adds to the overall compile time noticeably, and it was too small of a C file to be worth that overhead. (Compiling all of the headers takes most the time.)

Nice testing by the way.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Dec 1, 2012


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-12-01T21:41:23Z


In 3.843.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.