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

Initial implementation of elaboration system tasks #983

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

udif
Copy link
Contributor

@udif udif commented May 3, 2019

(IEEE1800-2017 section 20.11)
This PR allows us to use $info/$warning/$error/$fatal at elaboration time within a generate block.
This is very useful to stop a synthesis of a parametrized block when an illegal combination of parameters is chosen.

Some notes regarding this PR:

  1. While it is not 100% complete, it is functional, and provides a useful enhancement as-is.
  2. I added a new parser token, TOK_ELAB_TASK to prevent shift/reduce conflicts beween module instanciations within generate blocks and the new system tasks.
  3. I added a new AST node, AST_TECALL, so that elaboration system tasks won't get handled by normal system task calls.
  4. I chose not to add it within Ast::simplify() because this is a huge functiion, and I'm not sure I won't break anything. The use of a new AST node certainly helps avoiding this.

Known limitations:

  1. Test runs, but I don't know how to fail the regression if the test output is not as expected.
  2. Test doesn't check for $error and $fatal since it will fail the test regression.
  3. $info/$warning do not handle the full parameters as $display() does, only a simple message. We need to add a full $display() parser for that, perhaps take it from another Verilog open source codebase with an appropriate license. In addition, we might want to call simplify() on AST_TECALL's children in case some expressions appear as part of the tasks' parameters.
  4. $info/$warning are tested within if/case/for statements in a generate blocks, but are silently ignoredwhen put directly in a generate block with none of the enclosing statements above.

Please see corresponding Verilator bug:
https://www.veripool.org/issues/1429-Verilator-Feature-request-elaboration-tasks

(IEEE1800-2017 section 20.11)
This PR allows us to use $info/$warning/$error/$fatal **at elaboration time** within a generate block.
This is very useful to stop a synthesis of a parametrized block when an
illegal combination of parameters is chosen.
@udif
Copy link
Contributor Author

udif commented May 14, 2019

@cliffordwolf , can you please take a look at this? verilog_parser.y and verilog_lexer.l are the target of many PR's and the longer you delay looking at this, means more PR's will get out of sync.

I have several potential PR's to advance the state of SV language input I would like to write and submit, but they all modify the parser & lexer, so I have no wish to work on multiple PR's in parallel.

However, when my PR is not looked at for a long time, nor even acknowledged with message such as "saw your PR, too busy to comment right now, will take a look next week/month/whatever",
it doesn't inspire me to put more of my limited free time into this.

The more this delayed, the higher the chance other PRs will cause conflicts for my PR - in fact, it already does - it did not conflict when I submitted it.

@mithro
Copy link
Contributor

mithro commented May 14, 2019

I would like to support the addition of these features, I tried to use them recently and was saddened to find out they didn't work.

@ZipCPU eventually suggested the following work around version;
https://gist.github.com/ZipCPU/2eee912838d67eeec2e021f5351f54f6

Copy link
Collaborator

@cliffordwolf cliffordwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments JFYI. See #1077 for fixed and updated version.

va_start(ap, format);
std::string prefix = stringf("%s:%d: Info: ",
filename.c_str(), lineno);
logv_warning_with_prefix(prefix.c_str(), format, ap);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think $info should be printed as warning.

@@ -1146,7 +1146,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,

if (type == AST_GENFOR) {
for (size_t i = 0; i < buf->children.size(); i++) {
buf->children[i]->simplify(false, false, false, stage, -1, false, false);
if (!buf->children[i]->check_elab_tasks())
buf->children[i]->simplify(false, false, false, stage, -1, false, false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the right thing to do this. Consider the following example:

module test;
	$info({"foo", "bar"});
endmodule

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 this pull request may close these issues.

3 participants