Smoke test improvement roadmap

Introduction

The smoke-tests have around 30000 lines of error output, mostly regarding syntax errors, macro problems and header inclusion issues. Around 1400 of those lines are not yet categorized. As an example, the opentitan test analysis report shows:

Name	Count
unresolved-macro	21
slang-verified-error	16
related-to-slang-validated-error	1056
standalone-header	571
misc-preprocessor	3
likely-unhandled-macro-call	7
related-to-likely-unhandled-macro-call	272
misc-preprocessor-related	19
undefined	13
All	1978

To explain briefly the more miscellaneous categories, slang-verified-error means that the issue detected in smoke-tests had also been found with slang. related-to-* means that error * had subsequent issues that were related to the existence of a syntax error previously. This behavior was tested manually and I have found that the subsequent issues disappear when the root cause was dealt with.

Problems

As it can be clearly seen, the main issues are related to the preporcessor, mostly:

Mishandling macro calls, which in some parts of the syntax cause the issues to break the understanding of the rest of the file, eg. in ibex there is an enum with a macro that produces syntax errors at the macro itself:


typedef enum bit [5:0] {
LOAD = 0,
STORE,
SHIFT,
ARITHMETIC,
LOGICAL,
COMPARE,
BRANCH,
JUMP,
SYNCH,
SYSTEM,
COUNTER,
CSR,
CHANGELEVEL,
TRAP,
INTERRUPT,
`VECTOR_INCLUDE("riscv_instr_pkg_inc_riscv_instr_category_t.sv")
AMO // (last one)
} riscv_instr_category_t;


 Another use case where macro calls produce lots of syntax errors is visible in `black-parrot`, where parameters are declared using macros. There, the syntax tool throws 95 errors in a file starting with this declaration (`bp_be/src/v/bp_be_calculator/bp_be_calculator_top.sv`):

```systemverilog
module bp_be_calculator_top
 import bp_common_pkg::*;
 import bp_be_pkg::*;
 #(parameter bp_params_e bp_params_p = e_bp_default_cfg
    `declare_bp_proc_params(bp_params_p)
    `declare_bp_core_if_widths(vaddr_width_p, paddr_width_p, asid_width_p, branch_metadata_fwd_width_p)
    `declare_bp_cache_engine_if_widths(paddr_width_p, dcache_ctag_width_p, dcache_sets_p, dcache_assoc_p, dword_width_gp, dcache_block_width_p, dcache_fill_width_p, dcache)

// ...and so it continues

Where the problem is detected immediately (line 22 is the first macro of the parameter list) at:

/tmp/test/verible-smoke-test/black-parrot/bp_be/src/v/bp_be_calculator/bp_be_calculator_top.sv:22:5-27: syntax error at token "`declare_bp_proc_params"

and subsequently there are 94 other syntax errors unrelated to the macro all over the file. Removing all macros from parameter declarations showed that the file contains no errors in the rest of the code.

Not resolving macros across a project / body of code / libraries (unresolved macro error messages). Also visible widely in black-parrot, the issue probably stems from the filelists containing only the analyzed file and not looking at the project structure in any way. In the same file from above (bp_be/src/v/bp_be_calculator/bp_be_calculator_top.sv), lint tool returns a problem:
```
/tmp/test/verible-smoke-test/black-parrot/bp_be/src/v/bp_be_calculator/bp_be_calculator_top.sv:22:5-27: preprocessing error at token "`declare_bp_proc_params" : Error expanding macro identifier, might not be defined before.
```
Trying to parse .svh (or sometimes just .sv) headers outside of its designated context, causing a lot of syntax issues (eg. in opentitan hw/dv/sv/cip_lib/seq_lib/cip_base_vseq__sec_cm_fi.svh is included into dv/sv/cip_lib/seq_lib/cip_base_vseq.sv ). This is also due to not keeping proper project file lists and focusing on singular files without any context and structure.
Having preprocessor flow control cause problems. Here, one of the minor reported issues causes a syntax error right after a macro (this time not the macro itself is causing problems; see #1837)

Other than that, most of the issues are present in other tools as well (here, the slang suite was used as a reference due to it performing exceedingly well in ivtests) or are not categorized. Now over 90% of the volume of problems is categorized and attributed to issues or verified to be valid.

Severity

Looking at the sheer number of affected lines, the most glaring issues are:

getting confused and ultimately failing more when encountering a syntax error, especially when concerning macro calls within module parameters,
not dealing with files that are designated to be included into other files; best to actually include them and process the whole result, otherwise just ignore the contents or wrap those in a structure that allows arbitrary code inside of it. The best solution would be a complete project handling toolkit that would link all the includes together, but it is a rather large task.

Other, smaller issues found while combing through smoke tests are:

sequences not handling @ tokens ( #1838 )
preprocessor control flow produces in Core-VeeR-EH2 issues regarding the first token right after a macro call inside an ifdef. The minimized test case is ( #1837 ):
```
`define MACRO .baz,
module foo(
input  bar,
input  baz,
input  jaz
);
`ifdef MACRO
foo bar(
.bar,
`MACRO
.jaz
);
`endif
endmodule`
```

chipsalliance / verible

[Discussion] Smoke Test improvement roadmap #1839

Smoke test improvement roadmap

Introduction

Problems

Severity