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

Introduce IEEE Std 1497-2001 Standard Delay Format (SDF) parsing #1083

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

likeamahoney
Copy link
Contributor

@likeamahoney likeamahoney commented Aug 7, 2024

Good day everyone! In order to be able to support restrictions from 32 LRM section and annotate the SystemVerilog code using SDF (the last and only standard is IEEE Std 1497-2001) constructs I implemented the LL(2) parser using the slang infrastructure.

I encourage testing on individual SDF files for feedback, as the current implementation has been tested on various open-source SDF files.

The new --sdf option in the slang infrastructure allows for feeding slang one or more SDF files and also directory with SDF files. The implementation supports the full SDF grammar (including the syntax of conditional and timing check SDF expressions which is significantly differs from SystemVerilog expressions), handling all possible syntax errors efficiently. Example of usage:

./slang --sdf=test1.sdf,test2.sdf tests.sv  // slang always expects verilog/systemverilog files as input

What has not been done at the moment:

  • now only translation into a syntax tree has been implemented (it seems to me that there is no need to translate into a separate AST but only to supplement the existing SystemVerilog nodes of the slang AST - but it questionable)
  • It is not clear how large the number of grammar tests should be
  • processing of SDF DELAYs constructs - is it necessary to directly put values ​​from SDF to SystemVerilog specify AST? or it should be done in simulation (possibly using some pseudo SDF annotator - which defined at LRM 32.3 section), and nothing should be changed in AST? LRM specifies no possible restrictions with SDF DELAYs
  • SDF TIMINGCHECKs constructs - slang timing check system tasks currently are not annotated - it is future work
  • SDF LABELs constructs - same questions as SDF DELAY section - is it need to propagate it's values directly in AST?
  • possible changes in $sdf_annotate processing - is required or not?

I suppose SDF TIMINGENV constructs should be ignored by slang as it said at LRM 32.3 section:

An SDF file can contain many constructs that are not related to specify path delays, specparam values,
timing check constraint values, or interconnect delays. An example is any construct in the TIMINGENV
section of the SDF file. All constructs unrelated to SystemVerilog timing shall be ignored without any
warnings issued.

An opened question pertains to annotating unique delays between source/load pairs using the INTERCONNECT construct. How this warning should be processed, either by slang or the SDF annotator, remains unclear:

In the case of multisource nets, unique delays can be annotated between each source/load pair using the
INTERCONNECT construct. When annotating this construct, the SDF annotator shall find the source port and
the load port; and if both exist, it shall annotate an interconnect delay between the two. If the source port is
not found or if the source port and the load port are not actually on the same net, then a warning is issued,
but the delay to the load port is annotated anyway

@likeamahoney likeamahoney changed the title Introduce IEEE Std 1497-2001 Standart Delay Format (SDF) parsing Introduce IEEE Std 1497-2001 Standard Delay Format (SDF) parsing Aug 7, 2024
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 74.38895% with 241 lines in your changes missing coverage. Please review.

Project coverage is 94.33%. Comparing base (6969c74) to head (2a3d8fa).
Report is 3 commits behind head on master.

Files Patch % Lines
source/parsing/Parser_members.cpp 85.90% 94 Missing ⚠️
source/parsing/LexerFacts.cpp 5.08% 56 Missing ⚠️
source/syntax/SyntaxFacts.cpp 21.05% 30 Missing ⚠️
source/syntax/SyntaxTree.cpp 0.00% 22 Missing ⚠️
source/driver/SourceLoader.cpp 0.00% 16 Missing ⚠️
source/driver/Driver.cpp 25.00% 9 Missing ⚠️
source/parsing/Parser_expressions.cpp 91.74% 9 Missing ⚠️
source/parsing/Lexer.cpp 76.92% 3 Missing ⚠️
include/slang/util/LanguageVersion.h 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1083      +/-   ##
==========================================
- Coverage   94.72%   94.33%   -0.39%     
==========================================
  Files         191      191              
  Lines       47712    48638     +926     
==========================================
+ Hits        45193    45882     +689     
- Misses       2519     2756     +237     
Files Coverage Δ
include/slang/driver/SourceLoader.h 100.00% <100.00%> (ø)
include/slang/parsing/Lexer.h 100.00% <100.00%> (ø)
include/slang/parsing/Parser.h 100.00% <100.00%> (ø)
include/slang/syntax/SyntaxTree.h 100.00% <ø> (ø)
source/ast/Scope.cpp 98.52% <ø> (ø)
include/slang/util/LanguageVersion.h 44.44% <0.00%> (-12.70%) ⬇️
source/parsing/Lexer.cpp 99.17% <76.92%> (-0.31%) ⬇️
source/driver/Driver.cpp 93.51% <25.00%> (-1.61%) ⬇️
source/parsing/Parser_expressions.cpp 97.97% <91.74%> (-0.72%) ⬇️
source/driver/SourceLoader.cpp 88.41% <0.00%> (-4.54%) ⬇️
... and 4 more

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6969c74...2a3d8fa. Read the comment docs.

Yan Churkin and others added 2 commits August 8, 2024 00:04
Standard Delay Format (SDF) parsing
@MikePopoloski
Copy link
Owner

Thanks for the PR. I'm curious about the approach of fully reusing the Lexer, Preprocessor, Parser, and SyntaxTree classes from Verilog to also parse SDF files. It results in a lot of additional branching along the way since syntax differs significantly between the two languages. It also leads to some weird side effects that I don't think you'd want, like for example it seems you could probably use SystemVerilog defines and macro expansion in SDF files? Or at the lexical level, the Lexer will accept a bunch of token kinds that aren't actually allowed in SDF files.

I think it would make more sense to create separate copies of these classes, that target just parsing SDF. If there is useful overlap between the two you can factor them into a common base; the ParseBase class kind of does that already for the parser, for example. SDF files are much simpler than SystemVerilog so the resulting classes would be much more compact.

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