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

Fix icestudio test-examples to pass 'apio lint'. #537

Open
zapta opened this issue Jan 11, 2025 · 11 comments
Open

Fix icestudio test-examples to pass 'apio lint'. #537

zapta opened this issue Jan 11, 2025 · 11 comments

Comments

@zapta
Copy link
Collaborator

zapta commented Jan 11, 2025

@cavearr, can you look into this?

the apio/test-examples dir contains a few tens of project that were generated with icestudio and they fails 'apio lint'. This issue is to fix them, for example, by generating again their apio projects with the latest icestudio.

Example 1

/projects/apio-dev/repo/test-examples/ice40/alhambra-ii$ apio lint -p icestudio-ledon/
Setting the envinronment.
[Sat Jan 11 12:37:01 2025] Processing alhambra-ii
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Scanning for issues.
Testbench ledon_tb.v
verilator_bin --lint-only --quiet --bbox-unsup --timing -Wno-TIMESCALEMOD -Wno-MULTITOP --top-module main -DNO_ICE40_DEFAULT_ASSIGNMENTS -I"/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40" _build/hardware.vlt "/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v" main.v ledon_tb.v
%Warning-ASCRANGE: main.v:8:9: Ascending bit range vector: left < right of bit range: [0:6]
                             : ... note: In instance 'main'
    8 |  output [0:6] vinit
      |         ^
                   ... For warning description see https://verilator.org/warn/ASCRANGE?v=5.031
                   ... Use "/* verilator lint_off ASCRANGE */" and lint_on around source to disable this message.
scons: *** [_build/hardware] Error 1
============================================================================= [ ERROR ] Took 0.23 seconds =============================================================================

Example 2

/projects/apio-dev/repo/test-examples/ice40/alhambra-ii$ apio lint -p icestudio-riscv-stop-watch/
Setting the envinronment.
[Sat Jan 11 12:37:07 2025] Processing alhambra-ii
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Scanning for issues.
Testbench 03-riscv-stop-watch_tb.v
verilator_bin --lint-only --quiet --bbox-unsup --timing -Wno-TIMESCALEMOD -Wno-MULTITOP --top-module main -DNO_ICE40_DEFAULT_ASSIGNMENTS -I"/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40" _build/hardware.vlt "/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v" main.v 03-riscv-stop-watch_tb.v
%Warning-PINMISSING: main.v:85:10: Cell has missing pin: 'v8d2eee'
   85 |  vf1cffe v468719 (
      |          ^~~~~~~
                     main.v:150:9: ... Location of port declaration
  150 |  output v8d2eee
      |         ^~~~~~~
                     ... For warning description see https://verilator.org/warn/PINMISSING?v=5.031
                     ... Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message.
%Warning-PINMISSING: main.v:125:10: Cell has missing pin: 'vb385cd'
  125 |  v04e061 vb15d38 (
      |          ^~~~~~~
                     main.v:2583:15: ... Location of port declaration
 2583 |  output [4:0] vb385cd,
      |               ^~~~~~~
%Warning-PINMISSING: main.v:125:10: Cell has missing pin: 'vd9f5b6'
  125 |  v04e061 vb15d38 (
      |          ^~~~~~~
                     main.v:2584:9: ... Location of port declaration
 2584 |  output vd9f5b6
      |         ^~~~~~~
%Warning-PINMISSING: main.v:626:10: Cell has missing pin: 'v51fb1f'
  626 |  v794b6d va8ea8d (
      |          ^~~~~~~
                     main.v:782:16: ... Location of port declaration
  782 |  output [21:0] v51fb1f,
      |                ^~~~~~~
%Warning-PINMISSING: main.v:630:10: Cell has missing pin: 'v7e4f0f'
  630 |  vaaf5c4 ve8e400 (
      |          ^~~~~~~
                     main.v:817:9: ... Location of port declaration
  817 |  output v7e4f0f
      |         ^~~~~~~
%Warning-PINMISSING: main.v:635:10: Cell has missing pin: 'v712289'
  635 |  vaaf5c4 v677471 (
      |          ^~~~~~~
                     main.v:814:8: ... Location of port declaration
  814 |  input v712289,
      |        ^~~~~~~
%Warning-PINMISSING: main.v:635:10: Cell has missing pin: 'v7e4f0f'
  635 |  vaaf5c4 v677471 (
      |          ^~~~~~~
                     main.v:817:9: ... Location of port declaration
  817 |  output v7e4f0f
      |         ^~~~~~~
%Warning-PINMISSING: main.v:960:10: Cell has missing pin: 'v3f8943'
  960 |  v9a2795 v666bdb (
      |          ^~~~~~~
                     main.v:1071:9: ... Location of port declaration
 1071 |  output v3f8943,
      |         ^~~~~~~
%Warning-PINMISSING: main.v:960:10: Cell has missing pin: 'v64d863'
  960 |  v9a2795 v666bdb (
      |          ^~~~~~~
                     main.v:1072:9: ... Location of port declaration
 1072 |  output v64d863
      |         ^~~~~~~
%Warning-PINMISSING: main.v:964:10: Cell has missing pin: 'v62a8c1'
  964 |  va7b832 ve316c5 (
      |          ^~~~~~~
                     main.v:1109:16: ... Location of port declaration
 1109 |  output [16:0] v62a8c1,
      |                ^~~~~~~
%Warning-PINMISSING: main.v:968:10: Cell has missing pin: 'vbdb2c8'
  968 |  vef0f91 v3ffece (
      |          ^~~~~~~
                     main.v:1143:15: ... Location of port declaration
 1143 |  output [9:0] vbdb2c8
      |               ^~~~~~~
%Warning-PINMISSING: main.v:990:10: Cell has missing pin: 'vfc82fb'
  990 |  ve500df vfe8608 (
      |          ^~~~~~~
                     main.v:1533:16: ... Location of port declaration
 1533 |  output [28:0] vfc82fb,
      |                ^~~~~~~
%Warning-PINMISSING: main.v:1593:10: Cell has missing pin: 'vdd0469'
 1593 |  v468a05 v4dcb81 (
      |          ^~~~~~~
                     main.v:1726:15: ... Location of port declaration
 1726 |  output [7:0] vdd0469,
      |               ^~~~~~~
%Warning-PINMISSING: main.v:1593:10: Cell has missing pin: 'v4ba85d'
 1593 |  v468a05 v4dcb81 (
      |          ^~~~~~~
                     main.v:1727:15: ... Location of port declaration
 1727 |  output [7:0] v4ba85d,
      |               ^~~~~~~
%Warning-PINMISSING: main.v:1593:10: Cell has missing pin: 'vf93ecb'
 1593 |  v468a05 v4dcb81 (
      |          ^~~~~~~
                     main.v:1728:15: ... Location of port declaration
 1728 |  output [7:0] vf93ecb,
      |               ^~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'LATCH_INPUT_VALUE'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:19:9: ... Location of port declaration
   19 |  input  LATCH_INPUT_VALUE,
      |         ^~~~~~~~~~~~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'CLOCK_ENABLE'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:20:9: ... Location of port declaration
   20 |  input  CLOCK_ENABLE ,
      |         ^~~~~~~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'INPUT_CLK'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:21:9: ... Location of port declaration
   21 |  input  INPUT_CLK,
      |         ^~~~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'OUTPUT_CLK'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:22:9: ... Location of port declaration
   22 |  input  OUTPUT_CLK,
      |         ^~~~~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'D_OUT_1'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:25:9: ... Location of port declaration
   25 |  input  D_OUT_1,
      |         ^~~~~~~
%Warning-PINMISSING: main.v:3803:6: Cell has missing pin: 'D_IN_1'
 3803 |    ) input_pin (
      |      ^~~~~~~~~
                     /Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v:27:9: ... Location of port declaration
   27 |  output D_IN_1
      |         ^~~~~~
%Warning-PINMISSING: main.v:4026:10: Cell has missing pin: 'va1a83a'
 4026 |  v8d4ef5 v25f434 (
      |          ^~~~~~~
                     main.v:4102:16: ... Location of port declaration
 4102 |  output [15:0] va1a83a,
      |                ^~~~~~~
%Error: main.v:755:6: Duplicate declaration of signal: 'data_out'
                    : ... note: ANSI ports must have type declared with the I/O (IEEE 1800-2023 23.2.2.2)
  755 |  reg data_out;
      |      ^~~~~~~~
        main.v:741:16: ... Location of original declaration
  741 |  output [31:0] data_out
      |                ^~~~~~~~
%Error: main.v:2134:6: Duplicate declaration of signal: 'q'
 2134 |  reg q = INI;
      |      ^
        main.v:2131:9: ... Location of original declaration
 2131 |  output q
      |         ^
%Error: main.v:3889:6: Duplicate declaration of signal: 'q'
 3889 |  reg q = INI;
      |      ^
        main.v:3886:9: ... Location of original declaration
 3886 |  output q
      |         ^
%Error: main.v:4089:6: Duplicate declaration of signal: 'q'
 4089 |  reg q = INI;
      |      ^
        main.v:4087:9: ... Location of original declaration
 4087 |  output q
      |         ^
%Error: main.v:4235:6: Duplicate declaration of signal: 'q'
 4235 |  reg q = INI;
      |      ^
        main.v:4233:9: ... Location of original declaration
 4233 |  output q
      |         ^
scons: *** [_build/hardware] Error 1
@cavearr
Copy link
Member

cavearr commented Jan 11, 2025

Hi @zapta, tomorrow i'll fix them, this is not a problem of Icestudio is for the verilog inside.

The examples are very old and yosys get more stricted year by year and this examples contains errors.

Don't worry tomorros i'll take this task and fix it

@zapta
Copy link
Collaborator Author

zapta commented Jan 11, 2025 via email

@cavearr
Copy link
Member

cavearr commented Jan 11, 2025

i'm on it now, bug some of this have many problems, i'll try to fix as soon as possible

@cavearr
Copy link
Member

cavearr commented Jan 12, 2025

@zapta , finally i could work on it and all is fixed. You could try to lint ;)

@zapta
Copy link
Collaborator Author

zapta commented Jan 12, 2025 via email

@zapta
Copy link
Collaborator Author

zapta commented Jan 12, 2025

Hi @cavearr, thanks for fixing it.

  1. Four examples lint with no problem. One still gives a lint error, I think because of a non ascii char in the var name.
+ apio lint -p icestudio-jumping-led
Setting the envinronment.
[Sat Jan 11 19:59:18 2025] Processing alhambra-ii
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Scanning for issues.
Testbench 02-jumping-LED_tb.v
Warning: [02-jumping-LED_tb.v] Using $dumpfile() in apio testbenches is not recomanded.
verilator_bin --lint-only --quiet --bbox-unsup --timing -Wno-TIMESCALEMOD -Wno-MULTITOP --top-module main -DNO_ICE40_DEFAULT_ASSIGNMENTS -I"/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40" _build/hardware.vlt "/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v" main.v 02-jumping-LED_tb.v
%Error: 02-jumping-LED_tb.v:29:9: syntax error, unexpected end of file, expecting ',' or ';'
   29 |  reg Bot  n;
      |         ^
%Error: Cannot continue
scons: *** [_build/hardware] Error 1
  1. The test benches trigger the apio warning below. It is recomanded not to use $dumpfile() since apio already set the correct dump file output in the command line. Can you remove it from the testbenchs icestudio generates?
Warning: [ledon_tb.v] Using $dumpfile() in apio testbenches is not recomanded.

@zapta
Copy link
Collaborator Author

zapta commented Jan 12, 2025

  1. Also, the examples fail apio test. For example
/projects/apio-dev/repo/test-examples/ice40/alhambra-ii/icestudio-leds-buttons$ apio test
Setting the envinronment.
[Sat Jan 11 20:09:03 2025] Processing alhambra-ii
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Scanning for issues.
Testbench 01-LEDs-buttons_tb.v
Warning: [01-LEDs-buttons_tb.v] Using $dumpfile() in apio testbenches is not recomanded.
iverilog -g2012 -o _build/01-LEDs-buttons_tb.out -DVCD_OUTPUT=_build/01-LEDs-buttons_tb -DNO_ICE40_DEFAULT_ASSIGNMENTS -I"/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40" "/Users/user/.apio/packages/oss-cad-suite/share/yosys/ice40/cells_sim.v" main.v 01-LEDs-buttons_tb.v
01-LEDs-buttons_tb.v:25: error: 'constant_Constant' has already been declared in this scope.
01-LEDs-buttons_tb.v:24:      : It was declared here as a parameter.
01-LEDs-buttons_tb.v:29: error: 'Button' has already been declared in this scope.
01-LEDs-buttons_tb.v:28:      : It was declared here as a variable.
01-LEDs-buttons_tb.v:31: error: 'LED' has already been declared in this scope.
01-LEDs-buttons_tb.v:30:      : It was declared here as a net.
01-LEDs-buttons_tb.v:33: error: 'LED' has already been declared in this scope.
01-LEDs-buttons_tb.v:30:      : It was declared here as a net.
01-LEDs-buttons_tb.v:34: error: 'LED' has already been declared in this scope.
01-LEDs-buttons_tb.v:30:      : It was declared here as a net.
scons: *** [_build/01-LEDs-buttons_tb.out] Error 5
================================================================================= [ ERROR ] Took 0.22 seconds =================================================================================
/projects/apio-dev/repo/test-examples/ice40/alhambra-ii/icestudio-leds-buttons$ 

@cavearr
Copy link
Member

cavearr commented Jan 12, 2025

Don' worry @zapta , i'm review today, i don't review the testbenches , i only had been generated from Icestudio but yesterday i didn't could test it.

Today i'm reviewing all of this stuff and in the way i refine Icestudio linter parser and improve the error management.

@cavearr
Copy link
Member

cavearr commented Jan 12, 2025

@zapta , it's done, examples fixed!

@zapta
Copy link
Collaborator Author

zapta commented Jan 12, 2025

Thanks Carlos!

@zapta
Copy link
Collaborator Author

zapta commented Jan 13, 2025

Hi @cavearr, try to run this bash script at the root of the apio repo. It will test all the icestudio projects in test-examples, and will stop on first error.

BTW, I think that we have too many icestudio examples there so please feel free to delete some, this is also a form of fixing.

#!/bin/bash

# Exit on first error.
set -e

# Change to repo's dir.
cd ..

# Find all apio.ini in icestudio projects
projects=$(find test-examples | grep icestudio | grep apio.ini)

for proj in $projects;  do
    # Get the parent directory of apio.ini
    proj="$(dirname "$proj")"

    echo
    echo "--- PROJECT  $proj"
    echo

    # -- Go to the project's dir.
    pushd $proj

    # -- Test if the project has testbenches.
    set +e
    ls *_tb.v
    TB_STATUS=$?
    set -e


    # -- Exceute apio commands in the project. They should succeeed.
    set -x
      apio clean
      apio build
      apio lint
      if [ $TB_STATUS -eq 0 ]; then
        apio test
      fi   
      apio graph
      apio report
      apio clean
    set +x

    # -- Go back to the repo's root.
    popd
    
done

EDIT: Fixed the script above.

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

No branches or pull requests

2 participants