Linter(Verilator) enhancements (The-OpenROAD-Project#1837)

~ 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.
MrShaTr · Jun 19, 2023 · a3c416c · a3c416c
1 parent 66310e8
commit a3c416c
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 29 deletions.
diff --git a/.github/workflows/tool_updater.yml b/.github/workflows/tool_updater.yml
@@ -12,7 +12,7 @@ jobs:
     runs-on: ubuntu-20.04
     strategy:
       matrix:
-        tools: [magic, netgen, yosys, openroad_app]
+        tools: [magic, netgen, yosys, openroad_app, verilator]
     steps:
       - uses: actions/checkout@v2
         with:

diff --git a/configuration/checkers.tcl b/configuration/checkers.tcl
@@ -17,9 +17,10 @@ set ::env(QUIT_ON_ASSIGN_STATEMENTS) 0
 set ::env(QUIT_ON_UNMAPPED_CELLS) 1
 set ::env(QUIT_ON_SYNTH_CHECKS) 1
 set ::env(SYNTH_CHECKS_ALLOW_TRISTATE) 1
-set ::env(VERILATOR_RELATIVE_INCLUDES) 1
-set ::env(QUIT_ON_VERILATOR_WARNINGS) 0
-set ::env(QUIT_ON_VERILATOR_ERRORS) 1
+set ::env(LINTER_RELATIVE_INCLUDES) 1
+set ::env(LINTER_INCLUDE_PDK_MODELS) 0
+set ::env(QUIT_ON_LINTER_WARNINGS) 0
+set ::env(QUIT_ON_LINTER_ERRORS) 1
 
 # STA
 set ::env(QUIT_ON_TIMING_VIOLATIONS) 1

diff --git a/configuration/general.tcl b/configuration/general.tcl
@@ -24,7 +24,7 @@ set ::env(LEC_ENABLE) 0
 set ::env(YOSYS_REWRITE_VERILOG) 0
 set ::env(RUN_FILL_INSERTION) 1
 set ::env(RUN_TAP_DECAP_INSERTION) 1
-set ::env(RUN_VERILATOR) 1
+set ::env(RUN_LINTER) 1
 
 ## Intentionally Undocumented
 set ::env(RSZ_USE_OLD_REMOVER) 0

diff --git a/docker/verilator/Dockerfile b/docker/verilator/Dockerfile
@@ -26,11 +26,6 @@ RUN curl -L ${VERILATOR_REPO}/tarball/${VERILATOR_COMMIT} | tar -xzC . --strip-c
     make -j$(nproc)&&\
     make install
 
-# workaround for
-# %Error: //build/share/verilator/include/verilated_std.sv:28:9: Redeclaring the 'std' package is not allowed
-#28 | package std;
-#/build/share/verilator/include/verilated_std.sv:2:1: ... note: In file included from verilated_std.sv
-
 RUN rm /build/share/verilator/include/verilated_std.sv &&\
     touch /build/share/verilator/include/verilated_std.sv
 

diff --git a/docs/source/reference/configuration.md b/docs/source/reference/configuration.md
@@ -67,7 +67,6 @@ These variables are optional that can be specified in the design configuration f
 | `VERILOG_INCLUDE_DIRS` | Specifies the verilog includes directories. <br> Optional. |
 | `SYNTH_FLAT_TOP` | Specifies whether or not the top level should be flattened during elaboration. 1 = True, 0= False <br> (Default: `0` )|
 | `IO_PCT` | Specifies the percentage of the clock period used in the input/output delays. Ranges from 0 to 1.0. <br> (Default: `0.2`) |
-| `VERILATOR_RELATIVE_INCLUDES` | When a file references an include file, resolve the filename relative to the path of the referencing file, instead of relative to the current directory. <br> (Default: `1`) |
 
 ### STA
 
@@ -324,7 +323,7 @@ These variables worked initially, but they were too sky130 specific and will be
 | `DIODE_ON_PORTS` | Insert diodes on ports with the specified polarities. Available options are `none`, `in`, `out` and `both`. <br> (Default: `none`) |
 | `HEURISTIC_ANTENNA_THRESHOLD` | Minimum manhattan distance of a net to insert a diode in microns. Only applicable for `RUN_HEURISTIC_DIODE_INSERTION` is enabled. <br> (Default: `90`)
 | `USE_ARC_ANTENNA_CHECK` | Specifies whether to use the openroad ARC antenna checker or magic antenna checker. 0=magic antenna checker, 1=ARC OR antenna checker <br> (Default: `1`)
-| `RUN_VERILATOR` | Enable Verilator <br> (Default: `1`)
+| `RUN_LINTER` | Enable linter (currently Verilator) <br> (Default: `1`)
 | `TAP_DECAP_INSERTION` | **Deprecated: Use `RUN_TAP_DECAP_INSERTION`** Enables tap and decap cells insertion after floorplanning (if enabled) .1 = Enabled, 0 = Disabled <br> (Default: `1`) |
 | `MAGIC_CONVERT_DRC_TO_RDB` | **Removed: Will always run** Specifies whether or not generate a Calibre RDB out of the magic.drc report. Result is saved in `<run_path>/results/magic/`. 1=enabled 0=disabled <br> Default: `1`|
 | `TEST_MISMATCHES` | **Removed: See `./flow.tcl -test_mismatches`** Test for mismatches between the OpenLane tool versions and the current environment. `all` tests all mismatches. `tools` tests all except the PDK. `pdk` only tests the PDK. `none` disables the check.<br> (Default: `all`) |
@@ -349,8 +348,11 @@ These variables worked initially, but they were too sky130 specific and will be
 | `QUIT_ON_HOLD_VIOLATIONS ` | Exits the flow on hold violations at the typical corner <br> (Default: `1`)|
 | `QUIT_ON_SETUP_VIOLATIONS ` | Exits the flow on setup violations at the typical corner <br> (Default: `1`)|
 | `QUIT_ON_TIMING_VIOLATIONS ` | Controls `QUIT_ON_HOLD_VIOLATIONS` and `QUIT_ON_SETUP_VIOLATIONS` <br> (Default: `1`)|
-| `QUIT_ON_VERILATOR_WARNINGS` | Quit on warnings generated by Verilator <br> (Default: `0`)|
-| `QUIT_ON_VERILATOR_ERRORS` | Quit on errors generated by Verilator <br> (Default: `1`)|
+| `QUIT_ON_LINTER_WARNINGS` | Quit on warnings generated by linter (currently Verilator) <br> (Default: `0`)|
+| `QUIT_ON_LINTER_ERRORS` | Quit on errors generated by linter (currently Verilator) <br> (Default: `1`)|
+| `LINTER_RELATIVE_INCLUDES` | When a file references an include file, resolve the filename relative to the path of the referencing file, instead of relative to the current directory. <br> (Default: `1`) |
+| `LINTER_INCLUDE_PDK_MODELS` | Enables including verilog models of the pdk with the linter. This is useful when the design has hand instantiated macros. This variable has no effect if the PDK/STD_CELL_LIBRARY aren't supported. Currently, sky130A/sky130_fd_sc_hd and sky130B/sky130_fd_sc_hd are the only ones supported <br> (Default: `1`) |
+| `LINTER_DEFINES` | A list of defines that are passed to the linter. The syntax for each item in the list is as follows `<define>(=<value>)`. `(=value)` is optional. Both `PnR=1` or `PnR` are accepted <br> (Default: `SYNTH_DEFINES`) |
 
 
 ### On comma-delimited variables

diff --git a/flow.tcl b/flow.tcl
@@ -144,7 +144,7 @@ proc run_timing_check_step {args} {
 }
 
 proc run_verilator_step {} {
-    if { $::env(RUN_VERILATOR) } {
+    if { $::env(RUN_LINTER) } {
         run_verilator
     }
 }

diff --git a/scripts/tcl_commands/all.tcl b/scripts/tcl_commands/all.tcl
@@ -653,6 +653,10 @@ proc prep {args} {
     handle_deprecated_config FP_PDN_UPPER_LAYER FP_PDN_HORIZONTAL_LAYER
     handle_deprecated_config FP_PDN_LOWER_LAYER FP_PDN_VERTICAL_LAYER
     handle_deprecated_config PDN_CFG FP_PDN_CFG
+    handle_deprecated_config QUIT_ON_VERILATOR_WARNINGS QUIT_ON_LINTER_WARNINGS
+    handle_deprecated_config QUIT_ON_VERILATOR_ERRORS QUIT_ON_LINTER_ERRORS
+    handle_deprecated_config RUN_VERILATOR RUN_LINTER
+    handle_deprecated_config VERILATOR_RELATIVE_INCLUDES LINTER_RELATIVE_INCLUDES
 
     handle_diode_insertion_strategy
 

diff --git a/scripts/tcl_commands/synthesis.tcl b/scripts/tcl_commands/synthesis.tcl
@@ -250,26 +250,40 @@ proc run_verilator {} {
     if { [string match *$::env(PDK)* $verilator_verified_pdks] == 0 || \
         [string match *$::env(STD_CELL_LIBRARY)* $verilator_verified_scl] == 0} {
         puts_warn "PDK '$::env(PDK)', SCL '$::env(STD_CELL_LIBRARY)' will generate errors with instantiated stdcells in the design."
-        puts_warn "Either disable QUIT_ON_VERILATOR_ERRORS or remove the instantiated cells."
+        puts_warn "Either disable QUIT_ON_LINTER_ERRORS or remove the instantiated cells."
     } else {
         set pdk_verilog_models [glob $::env(PDK_ROOT)/$::env(PDK)/libs.ref/$::env(STD_CELL_LIBRARY_OPT)/verilog/*.v]
         foreach model $pdk_verilog_models {
-            set includes "$includes -I $model"
+            set includes "$includes $model"
         }
     }
-    set log $::env(synthesis_logs)/verilator.log
-    puts_info "Running Verilator (log: [relpath . $log])..."
+    set log $::env(synthesis_logs)/linter.log
+    puts_info "Running linter (Verilator) (log: [relpath . $log])..."
     set arg_list [list]
-    lappend arg_list {*}$includes
+    if { $::env(LINTER_INCLUDE_PDK_MODELS) } {
+        lappend arg_list {*}$includes
+    }
     lappend arg_list {*}$::env(VERILOG_FILES)
     if { [info exists ::env(VERILOG_FILES_BLACKBOX)] } {
         lappend arg_list {*}$::env(VERILOG_FILES_BLACKBOX)
     }
     lappend arg_list -Wno-fatal
-    if { $::env(VERILATOR_RELATIVE_INCLUDES) } {
+    if { $::env(LINTER_RELATIVE_INCLUDES) } {
         lappend arg_list "--relative-includes"
     }
 
+    set defines ""
+    if { [info exists ::env(LINTER_DEFINES)] } {
+        foreach define $::env(LINTER_DEFINES) {
+            set defines "$defines +define+$override"
+        }
+    } elseif { [info exists ::env(SYNTH_DEFINES)] } {
+        foreach define $::env(SYNTH_DEFINES) {
+            set defines "$defines +define+$override"
+        }
+    }
+    lappend arg_list {*}$defines
+
     set arg "|& tee $log $::env(TERMINAL_OUTPUT)"
     lappend arg_list {*}$arg
     try_exec bash -c "verilator \
@@ -279,25 +293,36 @@ proc run_verilator {} {
         --top-module $::env(DESIGN_NAME) \
         $arg_list"
 
+    set timing_errors [exec bash -c "grep -i 'Error-NEEDTIMINGOPT' $log || true"]
+    if { $timing_errors ne "" } {
+        set msg "Timing constructs found in the RTL. Please remove them or wrap them around an ifdef. It heavily unrecommended to rely on timing constructs for synthesis."
+        if { $::env(QUIT_ON_LINTER_ERRORS) } {
+            puts_err $msg
+            throw_error
+        } else {
+            puts_warn $msg
+        }
+    }
+
     set errors_count [exec bash -c "grep -i '%Error' $log | wc -l"]
     if { [expr $errors_count > 0] } {
-        if { $::env(QUIT_ON_VERILATOR_ERRORS) } {
-            puts_err "$errors_count errors found by Verilator"
+        if { $::env(QUIT_ON_LINTER_ERRORS) } {
+            puts_err "$errors_count errors found by linter"
             throw_error
         }
-        puts_warn "$errors_count errors found by Verilator"
+        puts_warn "$errors_count errors found by linter"
     } else {
-        puts_info "$errors_count errors found by Verilator"
+        puts_info "$errors_count errors found by linter"
     }
     set warnings_count [exec bash -c "grep -i '%Warning' $log | wc -l"]
     if { [expr $warnings_count > 0] } {
-        if { $::env(QUIT_ON_VERILATOR_WARNINGS) } {
-            puts_err "$warnings_count warnings found by Verilator"
+        if { $::env(QUIT_ON_LINTER_WARNINGS) } {
+            puts_err "$warnings_count warnings found by linter"
             throw_error
         }
-        puts_warn "$warnings_count warnings found by Verilator"
+        puts_warn "$warnings_count warnings found by linter"
     } else {
-        puts_info "$warnings_count warnings found by Verilator"
+        puts_info "$warnings_count warnings found by linter"
     }
 }