Skip to content

Assert that general always blocks have some timing control #1422

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AndrewNolte
Copy link
Contributor

From 9.2.2.1 General purpose always procedure:

"""
The always keyword represents a general purpose always procedure, which can be used to represent
repetitive behavior such as clock oscillators. The construct can also be used with proper timing controls to
represent combinational, latched, and sequential hardware behavior.

The general purpose always procedure, because of its looping nature, is only useful when used in
conjunction with some form of timing control. If an always procedure has no control for simulation time to
advance, it will create a simulation deadlock condition.

The following code, for example, creates a zero-delay infinite loop:
always areg = ~areg;
Providing a timing control to the preceding code creates a potentially useful description as shown in the
following:
always #half_period areg = ~areg;
"""

I've held off on updating all the failing tests because I want to get feedback on implementation first regarding where it should go:

  • Parser, where it is in this pr. The diag here seems a bit a premature, but the other option I saw was to place it in block symbols. This impl only operates on syntax, and so would either do redundant checks or need to cache results.
  • AnalysisManager, since technically that's correct, although this implementation is at the syntax level. It could also catch more cases with constant folding, but the major foot-guns are fairly simple as in the first example above.

@MikePopoloski
Copy link
Owner

Yeah, this needs to be further down the stack because of things like task calls:

module m;
  int i, j;
  task t;
    @* i = j;
  endtask
  
  always t();
  
  initial begin
    $monitor(i);
    #10 j = 3;
  end
endmodule

Probably it should go in the analysis pass.

@AndrewNolte
Copy link
Contributor Author

So would that be recording timings in handleTiming(), then checking at the end if there were any? Or does the call expression visitor not enter in yet and analyze the subproc?

@MikePopoloski
Copy link
Owner

Probably in AnalyzedProcedure, if it's an always block, you want to check if there is no timing and if so, check the collected CallExpressions to see if any called subroutines have timing. The getFunctionDrivers method gets AnalyzedProcedures for the called subroutines so you can extend something off of that.

@AndrewNolte AndrewNolte force-pushed the timing-control-in-always branch from 4d4c19f to bef08e6 Compare June 27, 2025 20:01
@AndrewNolte
Copy link
Contributor Author

I couldn't reuse getFunctionDrivers() since it only looks at functions not tasks- but the method I added ended up having a lot of the same logic. Should I split that logic out to just analyze all calls, then filter functions for driver aggregation, and tasks for the timing check?

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.

2 participants