From 210634586d2d6a205af8888d9a8697ba74c2957e Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Mon, 2 Mar 2020 15:14:36 +0000 Subject: [PATCH] Fix last verilator warning for ibex_simple_system; add waiver If you just build simple_system a fusesoc line like fusesoc --cores-root=. run --target=sim --setup \ --build lowrisc:ibex:ibex_simple_system then the change to ibex_simple_system.sv suffices, but if you explicitly set a parameter in fusesoc like this: fusesoc --cores-root=. run --target=sim --setup \ --build lowrisc:ibex:ibex_simple_system \ --RV32M=1 then it overrides the default parameter with a literal 1. We declare the parameter as an 'int', so I guess that's quite a reasonable behaviour from fusesoc. Anyway, this check only triggers when a 1-bit parameter is set with a literal 1, so should be safe. (If you do something buggy like setting it to 2, it will still moan at you). This patch adds a waiver file in examples/simple_system that silences the warning. This patch also makes the equivalent change to riscv_compliance, adding a waiver file in dv/riscv_compliance/lint and fixing up the default parameters. --- .../ibex_riscv_compliance.core | 6 ++++ dv/riscv_compliance/lint/verilator_waiver.vlt | 32 +++++++++++++++++++ .../rtl/ibex_riscv_compliance.sv | 6 ++-- .../simple_system/ibex_simple_system.core | 6 ++++ .../simple_system/lint/verilator_waiver.vlt | 32 +++++++++++++++++++ .../simple_system/rtl/ibex_simple_system.sv | 6 ++-- 6 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 dv/riscv_compliance/lint/verilator_waiver.vlt create mode 100644 examples/simple_system/lint/verilator_waiver.vlt diff --git a/dv/riscv_compliance/ibex_riscv_compliance.core b/dv/riscv_compliance/ibex_riscv_compliance.core index f8120ee5..f17ca31b 100644 --- a/dv/riscv_compliance/ibex_riscv_compliance.core +++ b/dv/riscv_compliance/ibex_riscv_compliance.core @@ -18,6 +18,11 @@ filesets: - rtl/riscv_testutil.sv file_type: systemVerilogSource + files_verilator_waiver: + files: + - lint/verilator_waiver.vlt: {file_type: vlt} + + parameters: RV32M: datatype: int @@ -39,6 +44,7 @@ targets: sim: default_tool: verilator filesets: + - tool_verilator ? (files_verilator_waiver) - files_sim_verilator parameters: - RV32M diff --git a/dv/riscv_compliance/lint/verilator_waiver.vlt b/dv/riscv_compliance/lint/verilator_waiver.vlt new file mode 100644 index 00000000..cf03755d --- /dev/null +++ b/dv/riscv_compliance/lint/verilator_waiver.vlt @@ -0,0 +1,32 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +// Lint waivers for processing riscv_compliance RTL with Verilator +// +// This should be used for rules applying to things like testbench +// top-levels. For rules that apply to the actual design (files in the +// 'rtl' directory), see verilator_waiver_rtl.vlt in the same +// directory. +// +// See https://www.veripool.org/projects/verilator/wiki/Manual-verilator#CONFIGURATION-FILES +// for documentation. +// +// Important: This file must included *before* any other Verilog file is read. +// Otherwise, only global waivers are applied, but not file-specific waivers. + +`verilator_config + +// We have some boolean top-level parameters in e.g. simple_system.sv. +// When building with fusesoc, these get set with defines like +// -GRV32M=1 (rather than -GRV32M=1'b1), leading to warnings like: +// +// Operator VAR '' expects 1 bits on the Initial value, but +// Initial value's CONST '32'h1' generates 32 bits. +// +// This signoff rule ignores errors like this. Note that it only +// matches when you set a 1-bit value to a literal 1, so it won't hide +// silly mistakes like setting it to 2. +// +lint_off -rule WIDTH -file "*/rtl/ibex_riscv_compliance.sv" + -match "*expects 1 bits*Initial value's CONST '32'h1'*" diff --git a/dv/riscv_compliance/rtl/ibex_riscv_compliance.sv b/dv/riscv_compliance/rtl/ibex_riscv_compliance.sv index 1a3c5570..78879b17 100644 --- a/dv/riscv_compliance/rtl/ibex_riscv_compliance.sv +++ b/dv/riscv_compliance/rtl/ibex_riscv_compliance.sv @@ -15,9 +15,9 @@ module ibex_riscv_compliance ( input IO_RST_N ); - parameter bit RV32E = 0; - parameter bit RV32M = 1; - parameter bit BranchTargetALU = 0; + parameter bit RV32E = 1'b0; + parameter bit RV32M = 1'b1; + parameter bit BranchTargetALU = 1'b0; logic clk_sys, rst_sys_n; diff --git a/examples/simple_system/ibex_simple_system.core b/examples/simple_system/ibex_simple_system.core index 0d3e6368..736367c5 100644 --- a/examples/simple_system/ibex_simple_system.core +++ b/examples/simple_system/ibex_simple_system.core @@ -17,6 +17,11 @@ filesets: - ibex_simple_system.cc: { file_type: cppSource } file_type: systemVerilogSource + files_verilator_waiver: + files: + - lint/verilator_waiver.vlt: {file_type: vlt} + + parameters: RV32M: datatype: int @@ -42,6 +47,7 @@ targets: sim: default_tool: verilator filesets: + - tool_verilator ? (files_verilator_waiver) - files_sim_verilator parameters: - RV32M diff --git a/examples/simple_system/lint/verilator_waiver.vlt b/examples/simple_system/lint/verilator_waiver.vlt new file mode 100644 index 00000000..1959a78b --- /dev/null +++ b/examples/simple_system/lint/verilator_waiver.vlt @@ -0,0 +1,32 @@ +// Copyright lowRISC contributors. +// Licensed under the Apache License, Version 2.0, see LICENSE for details. +// SPDX-License-Identifier: Apache-2.0 + +// Lint waivers for processing simple_system RTL with Verilator +// +// This should be used for rules applying to things like testbench +// top-levels. For rules that apply to the actual design (files in the +// 'rtl' directory), see verilator_waiver_rtl.vlt in the same +// directory. +// +// See https://www.veripool.org/projects/verilator/wiki/Manual-verilator#CONFIGURATION-FILES +// for documentation. +// +// Important: This file must included *before* any other Verilog file is read. +// Otherwise, only global waivers are applied, but not file-specific waivers. + +`verilator_config + +// We have some boolean top-level parameters in e.g. simple_system.sv. +// When building with fusesoc, these get set with defines like +// -GRV32M=1 (rather than -GRV32M=1'b1), leading to warnings like: +// +// Operator VAR '' expects 1 bits on the Initial value, but +// Initial value's CONST '32'h1' generates 32 bits. +// +// This signoff rule ignores errors like this. Note that it only +// matches when you set a 1-bit value to a literal 1, so it won't hide +// silly mistakes like setting it to 2. +// +lint_off -rule WIDTH -file "*/rtl/ibex_simple_system.sv" + -match "*expects 1 bits*Initial value's CONST '32'h1'*" diff --git a/examples/simple_system/rtl/ibex_simple_system.sv b/examples/simple_system/rtl/ibex_simple_system.sv index aa71e6f7..fd4abcbd 100644 --- a/examples/simple_system/rtl/ibex_simple_system.sv +++ b/examples/simple_system/rtl/ibex_simple_system.sv @@ -18,9 +18,9 @@ module ibex_simple_system ( input IO_RST_N ); - parameter bit RV32E = 0; - parameter bit RV32M = 1; - parameter bit BranchTargetALU = 0; + parameter bit RV32E = 1'b0; + parameter bit RV32M = 1'b1; + parameter bit BranchTargetALU = 1'b0; logic clk_sys = 1'b0, rst_sys_n;