From 51f7bf8948d5f5beac3b90576fb83ebe3fe21bff Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Sat, 4 May 2024 22:31:45 -0700 Subject: [PATCH 1/4] Improve performance of `verilog_library` by avoiding depset expansions --- cocotb/cocotb.bzl | 9 +++++---- synthesis/build_defs.bzl | 14 ++++++-------- verilator/defs.bzl | 28 ++++++++++----------------- verilator/tests/BUILD | 2 ++ verilog/providers.bzl | 21 +++++++++++++------- vivado/README.md | 2 ++ vivado/defs.bzl | 3 ++- vivado/tests/BUILD | 39 +++++++++++++++++++++++++------------- vivado/tests/gen_values.py | 12 ------------ 9 files changed, 67 insertions(+), 63 deletions(-) delete mode 100644 vivado/tests/gen_values.py diff --git a/cocotb/cocotb.bzl b/cocotb/cocotb.bzl index 8ab664d0..d92719b5 100644 --- a/cocotb/cocotb.bzl +++ b/cocotb/cocotb.bzl @@ -63,11 +63,12 @@ def _collect_verilog_files(ctx): verilog_info_struct.srcs for verilog_info_struct in transitive_srcs_depset.to_list() ] + verilog_data = [ + verilog_info_struct.data + for verilog_info_struct in transitive_srcs_depset.to_list() + ] - return depset( - [src for sub_tuple in verilog_srcs for src in sub_tuple] + - ctx.files.verilog_sources, - ) + return depset(transitive = verilog_srcs + verilog_data) def _collect_vhdl_files(ctx): return depset(direct = ctx.files.vhdl_sources) diff --git a/synthesis/build_defs.bzl b/synthesis/build_defs.bzl index 06958ab5..c5f57342 100644 --- a/synthesis/build_defs.bzl +++ b/synthesis/build_defs.bzl @@ -63,15 +63,15 @@ def _create_flist(ctx, flist_tag, files, short_path = False): def _synthesize_design_impl(ctx): transitive_srcs = _transitive_srcs([dep for dep in ctx.attr.deps if VerilogInfo in dep]) - verilog_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] - verilog_files = [src for sub_tuple in verilog_srcs for src in sub_tuple] - verilog_hdrs = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs.to_list()] - verilog_hdr_files = [hdr for sub_tuple in verilog_hdrs for hdr in sub_tuple] + verilog_srcs = depset(transitive = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()]) + verilog_data = depset(transitive = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()]) + verilog_files = depset(transitive = [verilog_srcs, verilog_data]) + verilog_hdrs = depset(transitive = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs.to_list()]) verilog_flist = _create_flist( ctx, flist_tag = "verilog", - files = verilog_files, + files = verilog_files.to_list(), ) uhdm_files = depset( @@ -98,8 +98,6 @@ def _synthesize_design_impl(ctx): abc_script = ctx.file.abc_script inputs = [] - inputs.extend(verilog_files) - inputs.extend(verilog_hdr_files) inputs.append(verilog_flist) inputs.append(uhdm_flist) inputs.extend(uhdm_files) @@ -178,7 +176,7 @@ def _synthesize_design_impl(ctx): ctx.actions.run( outputs = [output_file, log_file], - inputs = inputs, + inputs = depset(inputs, transitive = [verilog_hdrs, verilog_files]), arguments = [args], executable = ctx.executable.yosys_tool, env = env, diff --git a/verilator/defs.bzl b/verilator/defs.bzl index d8433d4b..198e3fd9 100644 --- a/verilator/defs.bzl +++ b/verilator/defs.bzl @@ -74,7 +74,10 @@ def cc_compile_and_link_static_library(ctx, srcs, hdrs, deps, runfiles, includes output_files.append(linking_output.library_to_link.dynamic_library) return [ - DefaultInfo(files = depset(output_files), runfiles = ctx.runfiles(files = runfiles)), + DefaultInfo( + files = depset(output_files), + runfiles = runfiles, + ), CcInfo( compilation_context = compilation_context, linking_context = linking_context, @@ -83,7 +86,6 @@ def cc_compile_and_link_static_library(ctx, srcs, hdrs, deps, runfiles, includes _CPP_SRC = ["cc", "cpp", "cxx", "c++"] _HPP_SRC = ["h", "hh", "hpp"] -_RUNFILES = ["dat", "mem"] def _only_cpp(f): """Filter for just C++ source/headers""" @@ -98,18 +100,9 @@ def _only_hpp(f): return None def _verilator_cc_library(ctx): - 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_files = [src for sub_tuple in all_srcs for src in sub_tuple] - - # Filter out .dat files. - runfiles = [] - verilog_files = [] - for file in all_files: - if file.extension in _RUNFILES: - runfiles.append(file) - else: - verilog_files.append(file) + dag = ctx.attr.module[VerilogInfo].dag + all_srcs = depset(transitive = [verilog_info_struct.srcs for verilog_info_struct in dag.to_list()]) + all_data = depset(transitive = [verilog_info_struct.data for verilog_info_struct in dag.to_list()]) verilator_output = ctx.actions.declare_directory(ctx.label.name + "-gen") @@ -123,15 +116,14 @@ def _verilator_cc_library(ctx): args.add("--prefix", prefix) if ctx.attr.trace: args.add("--trace") - for verilog_file in verilog_files: - args.add(verilog_file.path) + args.add_all(all_srcs) args.add_all(ctx.attr.vopts, expand_directories = False) ctx.actions.run( arguments = [args], mnemonic = "VerilatorCompile", executable = ctx.executable._verilator, - inputs = verilog_files, + inputs = depset(transitive = [all_srcs, all_data]), outputs = [verilator_output], progress_message = "[Verilator] Compiling {}".format(ctx.label), ) @@ -162,7 +154,7 @@ def _verilator_cc_library(ctx): srcs = [verilator_output_cpp], hdrs = [verilator_output_hpp], defines = defines, - runfiles = runfiles, + runfiles = ctx.runfiles(transitive_files = all_data), includes = [verilator_output_hpp.path], deps = deps, ) diff --git a/verilator/tests/BUILD b/verilator/tests/BUILD index cac3ca37..60fab2b9 100644 --- a/verilator/tests/BUILD +++ b/verilator/tests/BUILD @@ -47,6 +47,8 @@ verilog_library( name = "load_and_count", srcs = [ "load_and_count.sv", + ], + data = [ "test_data.dat", ], ) diff --git a/verilog/providers.bzl b/verilog/providers.bzl index 554bfb56..3968926d 100644 --- a/verilog/providers.bzl +++ b/verilog/providers.bzl @@ -24,7 +24,7 @@ VerilogInfo = provider( }, ) -def make_dag_entry(srcs, hdrs, deps, label): +def make_dag_entry(srcs, hdrs, data, deps, label): """Create a new DAG entry for use in VerilogInfo. As VerilogInfo should be created via 'merge_verilog_info' (rather than directly), @@ -39,15 +39,17 @@ def make_dag_entry(srcs, hdrs, deps, label): Args: srcs: A list of File that are 'srcs'. hdrs: A list of File that are 'hdrs'. + data: A list of File that are `data`. deps: A list of Label that are deps of this entry. label: A Label to use as the name for this entry. Returns: struct with all these fields properly stored. """ return struct( - srcs = tuple(srcs), - hdrs = tuple(hdrs), - deps = tuple(deps), + srcs = depset(srcs), + hdrs = depset(hdrs), + deps = depset(deps), + data = depset(data), label = label, ) @@ -69,7 +71,7 @@ def make_verilog_info( # dpis: Verilog DPI files. Returns: VerilogInfo that combines all the DAGs together. - """ + """ return VerilogInfo( dag = depset( direct = new_entries, @@ -91,6 +93,7 @@ def _verilog_library_impl(ctx): verilog_info = make_verilog_info( new_entries = [make_dag_entry( srcs = ctx.files.srcs, + data = ctx.files.data, hdrs = ctx.files.hdrs, deps = ctx.attr.deps, label = ctx.label, @@ -106,6 +109,10 @@ verilog_library = rule( doc = "Define a Verilog module.", implementation = _verilog_library_impl, attrs = { + "data": attr.label_list( + doc = "Compile data ready by sources.", + allow_files = True, + ), "deps": attr.label_list( doc = "The list of other libraries to be linked.", providers = [ @@ -114,11 +121,11 @@ verilog_library = rule( ), "hdrs": attr.label_list( doc = "Verilog or SystemVerilog headers.", - allow_files = True, + allow_files = [".vh", ".svh"], ), "srcs": attr.label_list( doc = "Verilog or SystemVerilog sources.", - allow_files = True, + allow_files = [".v", ".sv"], ), }, ) diff --git a/vivado/README.md b/vivado/README.md index ea3ba8ae..5aa27eec 100644 --- a/vivado/README.md +++ b/vivado/README.md @@ -1,5 +1,7 @@ # Vivado rules +Bazel rules for the [Vivado Design Suite](https://www.xilinx.com/developer/products/vivado.html). + The following are defined in `//vivado:defs.bzl`: * `vivado_create_project` diff --git a/vivado/defs.bzl b/vivado/defs.bzl index 6794ce91..c760fb44 100644 --- a/vivado/defs.bzl +++ b/vivado/defs.bzl @@ -98,7 +98,8 @@ def generate_file_load_tcl(module): """ transitive_srcs = depset([], transitive = [module[VerilogInfo].dag]) all_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] - all_files = [src for sub_tuple in all_srcs for src in sub_tuple] + all_data = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()] + all_files = depset(transitive = all_srcs + all_data).to_list() hdl_source_content, constraints_content, tcl_content = get_content_from_files(all_files) diff --git a/vivado/tests/BUILD b/vivado/tests/BUILD index 02b15264..b515f146 100644 --- a/vivado/tests/BUILD +++ b/vivado/tests/BUILD @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@bazel_skylib//rules:write_file.bzl", "write_file") load("@rules_cc//cc:defs.bzl", "cc_test") -load("@rules_python//python:defs.bzl", "py_binary") load("//verilator:defs.bzl", "verilator_cc_library") load("//verilog:defs.bzl", "verilog_library") load( @@ -33,6 +33,8 @@ verilog_library( name = "johnson_counter", srcs = [ "johnson_counter.sv", + ], + data = [ "test.mem", ], ) @@ -70,8 +72,10 @@ xsim_test( verilog_library( name = "johnson_counter_top", srcs = [ - "io_constraints.xdc", "johnson_counter_top.sv", + ], + data = [ + "io_constraints.xdc", "zcu111_gpio.tcl", ], deps = [ @@ -88,24 +92,31 @@ vivado_flow( xilinx_env = ":xilinx_env.sh", ) -py_binary( - name = "gen_values", - srcs = ["gen_values.py"], -) - -genrule( +write_file( name = "test_mem", - outs = ["test.mem"], - cmd = "$(location :gen_values) > $(OUTS)", - tools = [":gen_values"], + out = "test.mem", + content = [ + "00", + "05", + "0A", + "0F", + "14", + "19", + "1E", + "28", + "", + ], + newline = "unix", ) verilog_library( name = "weights_replay", srcs = [ - "test.mem", "weights_replay.sv", ], + data = [ + "test.mem", + ], ) verilator_cc_library( @@ -131,6 +142,8 @@ verilog_library( name = "weights_replay_top", srcs = [ "weights_replay_top.sv", + ], + data = [ "zcu111_weights.tcl", ], ) @@ -194,7 +207,7 @@ vivado_create_ip( verilog_library( name = "weights_replay_and_save_bd", - srcs = [ + data = [ "weights_replay_and_save_bd.tcl", ], ) diff --git a/vivado/tests/gen_values.py b/vivado/tests/gen_values.py deleted file mode 100644 index eab40afd..00000000 --- a/vivado/tests/gen_values.py +++ /dev/null @@ -1,12 +0,0 @@ - -VALUES = """00 -05 -0A -0F -14 -19 -1E -28""" - -if __name__ == "__main__": - print(VALUES) From 75923e5c24e8157a4afe59fb97afa16d94b724bb Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Tue, 7 May 2024 08:52:16 -0700 Subject: [PATCH 2/4] Revert VerilogInfo changes --- cocotb/cocotb.bzl | 9 ++++----- synthesis/build_defs.bzl | 14 ++++++++------ verilog/providers.bzl | 8 ++++---- vivado/defs.bzl | 3 +-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cocotb/cocotb.bzl b/cocotb/cocotb.bzl index d92719b5..8ab664d0 100644 --- a/cocotb/cocotb.bzl +++ b/cocotb/cocotb.bzl @@ -63,12 +63,11 @@ def _collect_verilog_files(ctx): verilog_info_struct.srcs for verilog_info_struct in transitive_srcs_depset.to_list() ] - verilog_data = [ - verilog_info_struct.data - for verilog_info_struct in transitive_srcs_depset.to_list() - ] - return depset(transitive = verilog_srcs + verilog_data) + return depset( + [src for sub_tuple in verilog_srcs for src in sub_tuple] + + ctx.files.verilog_sources, + ) def _collect_vhdl_files(ctx): return depset(direct = ctx.files.vhdl_sources) diff --git a/synthesis/build_defs.bzl b/synthesis/build_defs.bzl index c5f57342..06958ab5 100644 --- a/synthesis/build_defs.bzl +++ b/synthesis/build_defs.bzl @@ -63,15 +63,15 @@ def _create_flist(ctx, flist_tag, files, short_path = False): def _synthesize_design_impl(ctx): transitive_srcs = _transitive_srcs([dep for dep in ctx.attr.deps if VerilogInfo in dep]) - verilog_srcs = depset(transitive = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()]) - verilog_data = depset(transitive = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()]) - verilog_files = depset(transitive = [verilog_srcs, verilog_data]) - verilog_hdrs = depset(transitive = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs.to_list()]) + verilog_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] + verilog_files = [src for sub_tuple in verilog_srcs for src in sub_tuple] + verilog_hdrs = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs.to_list()] + verilog_hdr_files = [hdr for sub_tuple in verilog_hdrs for hdr in sub_tuple] verilog_flist = _create_flist( ctx, flist_tag = "verilog", - files = verilog_files.to_list(), + files = verilog_files, ) uhdm_files = depset( @@ -98,6 +98,8 @@ def _synthesize_design_impl(ctx): abc_script = ctx.file.abc_script inputs = [] + inputs.extend(verilog_files) + inputs.extend(verilog_hdr_files) inputs.append(verilog_flist) inputs.append(uhdm_flist) inputs.extend(uhdm_files) @@ -176,7 +178,7 @@ def _synthesize_design_impl(ctx): ctx.actions.run( outputs = [output_file, log_file], - inputs = depset(inputs, transitive = [verilog_hdrs, verilog_files]), + inputs = inputs, arguments = [args], executable = ctx.executable.yosys_tool, env = env, diff --git a/verilog/providers.bzl b/verilog/providers.bzl index 3968926d..91b288d1 100644 --- a/verilog/providers.bzl +++ b/verilog/providers.bzl @@ -46,10 +46,10 @@ def make_dag_entry(srcs, hdrs, data, deps, label): struct with all these fields properly stored. """ return struct( - srcs = depset(srcs), - hdrs = depset(hdrs), - deps = depset(deps), - data = depset(data), + srcs = tuple(srcs), + hdrs = tuple(hdrs), + data = tuple(data), + deps = tuple(deps), label = label, ) diff --git a/vivado/defs.bzl b/vivado/defs.bzl index c760fb44..6794ce91 100644 --- a/vivado/defs.bzl +++ b/vivado/defs.bzl @@ -98,8 +98,7 @@ def generate_file_load_tcl(module): """ transitive_srcs = depset([], transitive = [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()] - all_files = depset(transitive = all_srcs + all_data).to_list() + all_files = [src for sub_tuple in all_srcs for src in sub_tuple] hdl_source_content, constraints_content, tcl_content = get_content_from_files(all_files) From c6feece6ef29dce664284a0e2f85007d71034f10 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Tue, 7 May 2024 09:06:26 -0700 Subject: [PATCH 3/4] Restore data inputs to actions --- synthesis/build_defs.bzl | 3 ++- verilator/defs.bzl | 28 +++++++++++++++++++--------- vivado/defs.bzl | 3 ++- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/synthesis/build_defs.bzl b/synthesis/build_defs.bzl index 06958ab5..56039c3d 100644 --- a/synthesis/build_defs.bzl +++ b/synthesis/build_defs.bzl @@ -64,7 +64,8 @@ def _create_flist(ctx, flist_tag, files, short_path = False): def _synthesize_design_impl(ctx): transitive_srcs = _transitive_srcs([dep for dep in ctx.attr.deps if VerilogInfo in dep]) verilog_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] - verilog_files = [src for sub_tuple in verilog_srcs for src in sub_tuple] + verilog_data = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()] + verilog_files = [src for sub_tuple in (verilog_srcs + verilog_data) for src in sub_tuple] verilog_hdrs = [verilog_info_struct.hdrs for verilog_info_struct in transitive_srcs.to_list()] verilog_hdr_files = [hdr for sub_tuple in verilog_hdrs for hdr in sub_tuple] diff --git a/verilator/defs.bzl b/verilator/defs.bzl index 198e3fd9..7f442803 100644 --- a/verilator/defs.bzl +++ b/verilator/defs.bzl @@ -75,9 +75,7 @@ def cc_compile_and_link_static_library(ctx, srcs, hdrs, deps, runfiles, includes return [ DefaultInfo( - files = depset(output_files), - runfiles = runfiles, - ), + files = depset(output_files), runfiles = ctx.runfiles(files = runfiles)), CcInfo( compilation_context = compilation_context, linking_context = linking_context, @@ -86,6 +84,7 @@ def cc_compile_and_link_static_library(ctx, srcs, hdrs, deps, runfiles, includes _CPP_SRC = ["cc", "cpp", "cxx", "c++"] _HPP_SRC = ["h", "hh", "hpp"] +_RUNFILES = ["dat", "mem"] def _only_cpp(f): """Filter for just C++ source/headers""" @@ -100,9 +99,19 @@ def _only_hpp(f): return None def _verilator_cc_library(ctx): - dag = ctx.attr.module[VerilogInfo].dag - all_srcs = depset(transitive = [verilog_info_struct.srcs for verilog_info_struct in dag.to_list()]) - all_data = depset(transitive = [verilog_info_struct.data for verilog_info_struct in dag.to_list()]) + 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()] + all_files = [src for sub_tuple in (all_srcs + all_data) for src in sub_tuple] + + # Filter out .dat files. + runfiles = [] + verilog_files = [] + for file in all_files: + if file.extension in _RUNFILES: + runfiles.append(file) + else: + verilog_files.append(file) verilator_output = ctx.actions.declare_directory(ctx.label.name + "-gen") @@ -116,14 +125,15 @@ def _verilator_cc_library(ctx): args.add("--prefix", prefix) if ctx.attr.trace: args.add("--trace") - args.add_all(all_srcs) + for verilog_file in verilog_files: + args.add(verilog_file.path) args.add_all(ctx.attr.vopts, expand_directories = False) ctx.actions.run( arguments = [args], mnemonic = "VerilatorCompile", executable = ctx.executable._verilator, - inputs = depset(transitive = [all_srcs, all_data]), + inputs = verilog_files, outputs = [verilator_output], progress_message = "[Verilator] Compiling {}".format(ctx.label), ) @@ -154,7 +164,7 @@ def _verilator_cc_library(ctx): srcs = [verilator_output_cpp], hdrs = [verilator_output_hpp], defines = defines, - runfiles = ctx.runfiles(transitive_files = all_data), + runfiles = runfiles, includes = [verilator_output_hpp.path], deps = deps, ) diff --git a/vivado/defs.bzl b/vivado/defs.bzl index 6794ce91..f7e7a168 100644 --- a/vivado/defs.bzl +++ b/vivado/defs.bzl @@ -98,7 +98,8 @@ def generate_file_load_tcl(module): """ transitive_srcs = depset([], transitive = [module[VerilogInfo].dag]) all_srcs = [verilog_info_struct.srcs for verilog_info_struct in transitive_srcs.to_list()] - all_files = [src for sub_tuple in all_srcs for src in sub_tuple] + all_data = [verilog_info_struct.data for verilog_info_struct in transitive_srcs.to_list()] + all_files = [src for sub_tuple in (all_srcs + all_data) for src in sub_tuple] hdl_source_content, constraints_content, tcl_content = get_content_from_files(all_files) From 66264555e1600b6e3ab695744ad81317d81482cd Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Tue, 7 May 2024 09:08:57 -0700 Subject: [PATCH 4/4] buildifier --- verilator/defs.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/verilator/defs.bzl b/verilator/defs.bzl index 7f442803..affb98c6 100644 --- a/verilator/defs.bzl +++ b/verilator/defs.bzl @@ -75,7 +75,9 @@ def cc_compile_and_link_static_library(ctx, srcs, hdrs, deps, runfiles, includes return [ DefaultInfo( - files = depset(output_files), runfiles = ctx.runfiles(files = runfiles)), + files = depset(output_files), + runfiles = ctx.runfiles(files = runfiles), + ), CcInfo( compilation_context = compilation_context, linking_context = linking_context,