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 sampling #14

Closed
wants to merge 18 commits into from
Closed

Support sampling #14

wants to merge 18 commits into from

Conversation

kiryk
Copy link

@kiryk kiryk commented Aug 12, 2022

The change adds support for value sampling, i.e. access to values of expressions at the beginning of each time slot.

This is done with $sampled function except for concurrent assertions where sampling is done by default.
Before the change, the function was deleted during V3Assert pass when encountered in AST.

The change makes $past, $fell, $rose, $stable and $changed refer to sampled values, as expected by IEEE.
The change can simplify implementation of explicit clocking for these functions, but such functionality is not added here.

Sampling is implemented in two stages:

  1. On V3Assert stage the test expression in concurrent asserts is surrounded by a call to $sampled.
  2. On V3Clock state every call to $sampled is replaced by their only subexpression, and for every variable in the subexpression:
  • a helper variable holding the sampled value is created,
  • an assignment to the variable is added to the eval to occur at the beginning of every simulation step,
  • the original variable reference is replaced by a reference to the helper function.

The mechanic for that is based on the way such helper variables are created for clocks, in order to detect posedges/negedges.

vscp->fileline(), new AstVarRef(vscp->fileline(), newvscp, VAccess::WRITE),
new AstVarRef(vscp->fileline(), vscp, VAccess::READ));
m_evalFuncp->addInitsp(finalp);
//
Copy link
Member

Choose a reason for hiding this comment

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

Do we need that?

Copy link
Author

Choose a reason for hiding this comment

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

I don't think so, but it's placed here because it occurs in a corresponding place in the function used for clocks.

src/V3Clock.cpp Outdated
@@ -425,6 +447,26 @@ class ClockVisitor final : public VNVisitor {
addToEvalLoop(nodep);
}

// Replace $sampled calls with sampling variables
virtual void visit(AstSampled* nodep) override {
m_inSampled = true;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use VL_RESTORER here?

Copy link
Author

Choose a reason for hiding this comment

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

Done

Copy link

@kbieganski kbieganski left a comment

Choose a reason for hiding this comment

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

Nice and simple, though one thing I'm worried about is how mergeable this is with the V5 branch. V3Clock changed significantly there, so it might be an issue. If you can, please put that code in a different pass. If not, I suggest to try and rebase it and see if it takes a significant effort. If so, we should consider doing it and opening a PR against V5.

if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2003-2009 by Wilson Snyder. This program is free software; you

Choose a reason for hiding this comment

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

Suggested change
# Copyright 2003-2009 by Wilson Snyder. This program is free software; you
# Copyright 2022 by Antmicro Ltd. This program is free software; you

Same for the other test files.

Copy link
Author

Choose a reason for hiding this comment

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

I've modified the licenses. When it comes to compatibility with V5 - I think the effort to glue it will be significant, so I'm looking for another pass that could contain that code.

@kiryk kiryk force-pushed the kiryk/support-sampled branch from 1f0ad69 to b64a217 Compare August 12, 2022 14:53
@kbieganski kbieganski closed this Aug 24, 2022
@kiryk kiryk deleted the kiryk/support-sampled branch August 31, 2022 13:50
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