Skip to content

Fix slang errors and warnings in Koios benchmarks #3196

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

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

Conversation

petergrossmann21
Copy link
Contributor

Resolve slang errors/warnings in the Koios benchmarks.

Description

The errors/warnings fell into the following buckets:

  1. Add reg keyword to output ports where outputs are used in LHS of always blocks.
  2. Move timescale directives to top of files.
  3. Add empty output port connection declarations to eliminate warnings (no functional change)
  4. Change instance name to prevent namespace conflict with SystemVerilog keyword
  5. Comment out an initial block that used a variable that had also been commented out.

Related Issue

#3169

Motivation and Context

Eliminating slang errors allows these benchmarks to continue to be used in flows that make use of the yosys-slang plugin.

How Has This Been Tested?

Primary method of testing has been running slang directly at the command line. Quick research suggested there was not a single VTR regression test I could execute to try all four of these benchmarks in one push button; due to this and test runtime concerns I have deferred running VTR regression tests to validate that no synthesis results have changed.

From a synthesis standpoint, the risk of results changing due to these edits is very low.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added the lang-hdl Hardware Description Language (Verilog/VHDL) label Jul 10, 2025
@gadfort
Copy link

gadfort commented Jul 11, 2025

@petergrossmann21 @AlexandreSinger have any of these issues been reported to the yosys-slang project? Nothing in these changes seems like something it should have choked on, so it might be work verifying that the maintainer @povik is aware or that the VTR version of yosys-slang is kept up to date.

@povik
Copy link

povik commented Jul 11, 2025

@gadfort Now that you have pinged me I've taken a look. The two below are worth changing but I'd say it's of low priority.

  • Add reg keyword to output ports where outputs are used in LHS of always blocks.

This could be made into a demotable error.

  • Move timescale directives to top of files.

Ideally synthesis would ignore timescales. I suppose there was an error "time scale declaration must come before all other items in scope".

@petergrossmann21
Copy link
Contributor Author

@gadfort Now that you have pinged me I've taken a look. The two below are worth changing but I'd say it's of low priority.

  • Add reg keyword to output ports where outputs are used in LHS of always blocks.

This could be made into a demotable error.

  • Move timescale directives to top of files.

Ideally synthesis would ignore timescales. I suppose there was an error "time scale declaration must come before all other items in scope".

@povik regarding timescales you are correct that this is the error we see.

See also comments on #3195; what are your thoughts regarding handling parameters for modules read using previous read_verilog commands?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-hdl Hardware Description Language (Verilog/VHDL)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants