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.

Sign up for GitHub

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

Jump to bottom

Linter(Verilator) enhancements #1837

Merged
merged 9 commits into from
Jun 19, 2023
Merged

Linter(Verilator) enhancements #1837

merged 9 commits into from
Jun 19, 2023

Conversation

kareefardi
Copy link
Collaborator

@kareefardi kareefardi commented Jun 6, 2023 •
edited
Loading 

~ generalize verilator variables:
  QUIT_ON_VERILATOR_ERRORS -> QUIT_ON_LINTER_ERRORS
  QUIT_ON_VERILATOR_WARNINGS -> QUIT_ON_LINTER_WARNINGS
  VERILATOR_RELATIVE_INCLUDES -> LINTER_RELATIVE_INCLUDES
  RUN_VERILATOR -> RUN_LINTER
+ add LINTER_INCLUDE_PDK_MODELS
+ add LINTER_DEFINES
+ include verilator in ci tool updater
- do not include pdk verilog models using -I
- remove workaround for verilator std error
- disallow timing constructs. Print an error to the user to remove or guard them.

Supersedes #1836
Supersedes #1776
Fixes #1811
Fixes #1790

kareefardi added 3 commits June 6, 2023 16:05
@kareefardi
rename verilator variables:
ef7a1e7 
QUIT_ON_VERILATOR_ERRORS -> QUIT_ON_LINTER_ERRORS
QUIT_ON_VERILATOR_WARNINGS -> QUIT_ON_LINTER_WARNINGS
VERILATOR_RELATIVE_INCLUDES -> LINTER_RELATIVE_INCLUDES
RUN_VERILATOR -> RUN_LINTER

- Do not include pdk verilog models using -I
- Remove workaround for verilator std error
@kareefardi
more renames from verilator to linter
f481a61 
add LINTER_INCLUDE_PDK_MODELS
add LINTER_DEFINES
add LINTER_TIMING_CONSTRUCTS
@kareefardi
no need for --dump-defines
77394ad
@kareefardi
Copy link
Collaborator Author

TODO: remove LINTER_TIMING_CONSTRUCTS and disallow them

@kareefardi
Merge branch 'master' into verilator-fixes
ff38fae
@kareefardi
Grammar with logging message
4cf0532 
Disallow timing constructs and remove LINTER_TIMING_CONSTRUCTS
@donn donn mentioned this pull request Jun 13, 2023
Closed
@kareefardi kareefardi marked this pull request as ready for review June 13, 2023 11:47
@kareefardi kareefardi requested a review from donn June 13, 2023 17:29
donn
donn approved these changes Jun 18, 2023
View reviewed changes
Copy link
Collaborator

@donn donn left a comment

Choose a reason for hiding this comment

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

let me state for the record that i hate this

donn
donn requested changes Jun 18, 2023
View reviewed changes
scripts/tcl_commands/synthesis.tcl Outdated Show resolved Hide resolved
@kareefardi kareefardi requested a review from donn June 18, 2023 11:54
donn
donn previously approved these changes Jun 18, 2023
View reviewed changes
@kareefardi kareefardi requested a review from donn June 18, 2023 15:46
@kareefardi kareefardi merged commit a3c416c into The-OpenROAD-Project:master Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donn donn donn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OL quits on Verilator error, but they are only warnings Rename Verilator variables to be tool agnostic
2 participants
@kareefardi @donn