From d309aea67a9a48c187207188531a718154163d09 Mon Sep 17 00:00:00 2001 From: Filip Filmar Date: Mon, 11 Nov 2024 07:45:49 +0000 Subject: [PATCH] verilog: adds support for include headers Include headers should not appear in the verilator command line, but should be available in the compilation sandbox. It seems that verilator rules did not allow `hdrs` files into the sandbox, so this change fixes the issue. Furthermore, very often core authors use the include directive to read an include file from the directory where the include file is located: ``` `include "file.h" ``` This will not work in bazel, because the expected include path is: ``` `include "directory/where/file/resides/file.h" ``` This rule adds the `includes` parameter to `verilog_library` to allow the user to specify a set of include directories, therefore allowing verilation of included cores. The `verilator_cc_library` rules is fixed to suport this. It is also fixed to use the `hdrs` files as inputs to the verilation rule. --- verilator/defs.bzl | 24 +++++++++++++++++++++--- verilog/providers.bzl | 17 ++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/verilator/defs.bzl b/verilator/defs.bzl index c3e26d52..86b723db 100644 --- a/verilator/defs.bzl +++ b/verilator/defs.bzl @@ -104,10 +104,26 @@ def _verilator_cc_library(ctx): verilator_toolchain = ctx.toolchains["@rules_hdl//verilator:toolchain_type"] transitive_srcs = depset([], transitive = [ctx.attr.module[VerilogInfo].dag]) - all_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] - all_data = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()] + transitive_srcs_list = transitive_srcs.to_list() + all_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs_list] + all_data = [verilog_info_struct.data for verilog_info_struct in transitive_srcs_list] all_files = [src for sub_tuple in (all_srcs + all_data) for src in sub_tuple] + # Collect all directories named in the `includes` parameter. + all_include_tuples = [ + verilog_info_struct.includes + for verilog_info_struct in transitive_srcs_list] + all_includes = [ + include for sub_tuple in all_include_tuples for include in sub_tuple + ] + all_includes_unique = depset(all_includes).to_list() + + # hdrs should appear in the build sandbox, but *not* in the command line, + # so keep them separate, and don't add them to `all_files`. + all_hdrs_tuple = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs_list] + all_hdrs = [hdr for sub_tuple in all_hdrs_tuple for hdr in sub_tuple] + all_hdrs_unique = depset(all_hdrs).to_list() + # Filter out .dat files. runfiles = [] verilog_files = [] @@ -127,6 +143,8 @@ def _verilator_cc_library(ctx): args.add("--Mdir", verilator_output.path) args.add("--top-module", ctx.attr.module_top) args.add("--prefix", prefix) + # Verilator requires no space between `-I` and the directory name. + args.add_all(all_includes_unique, format_each = "-I%s") if ctx.attr.trace: args.add("--trace") for verilog_file in verilog_files: @@ -139,7 +157,7 @@ def _verilator_cc_library(ctx): mnemonic = "VerilatorCompile", executable = verilator_toolchain.verilator, tools = verilator_toolchain.all_files, - inputs = verilog_files, + inputs = verilog_files + all_hdrs_unique, outputs = [verilator_output], progress_message = "[Verilator] Compiling {}".format(ctx.label), ) diff --git a/verilog/providers.bzl b/verilog/providers.bzl index d8dc0077..d9124166 100644 --- a/verilog/providers.bzl +++ b/verilog/providers.bzl @@ -24,7 +24,7 @@ VerilogInfo = provider( }, ) -def make_dag_entry(srcs, hdrs, data, deps, label, tags): +def make_dag_entry(srcs, hdrs, data, deps, label, tags, includes): """Create a new DAG entry for use in VerilogInfo. As VerilogInfo should be created via 'merge_verilog_info' (rather than directly), @@ -43,6 +43,7 @@ def make_dag_entry(srcs, hdrs, data, deps, label, tags): deps: A list of Label that are deps of this entry. label: A Label to use as the name for this entry. tags: A list of str. (Ideally) just the entry tags for later filelist filtering. + includes: A list of additional include directories. Similar to the parameter of the same name in `cc_library`. The package path is prepended. Returns: struct with all these fields properly stored. """ @@ -53,6 +54,7 @@ def make_dag_entry(srcs, hdrs, data, deps, label, tags): deps = tuple(deps), tags = tuple(tags), label = label, + includes = tuple(includes), ) def make_verilog_info( @@ -92,6 +94,15 @@ def _verilog_library_impl(ctx): A struct containing the DAG at this level of the dependency graph. """ + full_includes = [] + for include in ctx.attr.includes: + if include[:2] == "//": + full_includes += [include[2:]] + elif include != "" and include != ".": + full_includes += ["/".join([ctx.label.package, dir])] + else: + full_includes += [dir] + verilog_info = make_verilog_info( new_entries = [make_dag_entry( srcs = ctx.files.srcs, @@ -100,6 +111,7 @@ def _verilog_library_impl(ctx): deps = ctx.attr.deps, label = ctx.label, tags = [], + includes = full_includes, )], old_infos = [dep[VerilogInfo] for dep in ctx.attr.deps], ) @@ -126,6 +138,9 @@ verilog_library = rule( doc = "Verilog or SystemVerilog headers.", allow_files = [".vh", ".svh"], ), + "includes": attr.string_list( + doc = "Include directories to add when necessary", + ), "srcs": attr.label_list( doc = "Verilog or SystemVerilog sources.", allow_files = [".v", ".sv"],