Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bazel] Allocate CW310s based on extra resources #16436

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

drewmacrae
Copy link
Contributor

@drewmacrae drewmacrae commented Nov 17, 2022

WIP requires custom bazel binary. Add's the extra resource "cw310" so that 1 test can be dispatched concurrently to other testing.

Signed-off-by: Drew Macrae drewmacrae@google.com

@drewmacrae
Copy link
Contributor Author

Tested with a bazel test //... --test_tag_filters=-broken

[3,766 / 3,860] 136 / 1364 tests; 75 actions, 7 running; last test: //third_party/riscv-compliance:I-SLLI-01_fpga_cw310_test_rom
    FuseSoC hw/build.verilator_real; 511s local, remote-cache
    Testing //third_party/riscv-compliance:I-SB-01_fpga_cw310_rom; 1s local
    Action sw/device/silicon_creator/lib/boot_data_functest_prog_sim_dv.fake_dev_key_0.signed.64.scr.vmem; 1s remote-cache, linux-sandbox
    Action sw/device/silicon_creator/lib/boot_data_functest_prog_sim_dv.logs.txt; 0s remote-cache, linux-sandbox
    [Sched] Testing //sw/device/silicon_creator/rom/e2e:e2e_bootstrap_entry_test_unlocked0_fpga_cw310_rom; 461s
    [Sched] Testing //third_party/riscv-compliance:I-LH-01_fpga_cw310_test_rom; 459s
    [Sched] Testing //sw/device/silicon_creator/rom/e2e:e2e_bootstrap_entry_rma_fpga_cw310_rom; 459s

Note verilator is building and FPGA tests are running

drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Nov 22, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
@drewmacrae
Copy link
Contributor Author

drewmacrae commented Nov 22, 2022

Using the "cpu:" tag to control verilator makes it harder to control the number of threads verilator tests are allocated at the command line. The machinery to build verilator for n threads is still in place, we can still build for 4 by default and tag verilator tests with "cpu:4", but bazelbuild/bazel#16786 keeps us from limiting the total number of tests HOST_CPUs/verilator_threads when we change the number of threads verilator is built for.

It's a hack but I'm using:

#build verilator for 8 threads instead of 4 for a ~30% speedup
build:fast --//hw:verilator_options=--threads,8

#8 thread verilator gets a nice speedup, but test_jobs break resource management r/n and jobs slows things down pretty significantly. 
#build:fast --jobs=HOST_CPUS*0.11
#build:fast --local_test_jobs=HOST_CPUS*0.11
build:fast --local_cpu_resources=HOST_CPUS*0.5

@drewmacrae
Copy link
Contributor Author

there are some rstmgr and alert_handler tests that are a bit flaky when tests aren't exclusive. I think there are some race conditions. I've seen some failures in:
//sw/device/tests:rstmgr_alert_info_test_fpga_cw310_test_rom
//sw/device/tests:alert_handler_reverse_ping_in_deep_sleep_test_fpga_cw310_test_rom
//sw/device/tests:alert_handler_ping_timeout_test_fpga_cw310_rom

drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Nov 30, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
@drewmacrae
Copy link
Contributor Author

Decreases our test suite from 345m down to 188m, but running tests in parallel means we can catch new errors orders of magnitude faster.

drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Dec 10, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Dec 10, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Dec 10, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Dec 10, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Dec 10, 2022
Addressed comments on bazelbuild#13996
Fixed issues in tests and built and tested with
lowRISC/opentitan#16436

Signed-off-by: Drew Macrae <drewmacrae@google.com>
WIP requires custom bazel binary. Don't merge until bazel is updated to
reflect upstream changes.

Removes the external tag in preference for a custom resource as handled
by bazelbuild/bazel#16785

Signed-off-by: Drew Macrae <drewmacrae@google.com>
@drewmacrae
Copy link
Contributor Author

profile2.gz
This is a result of the --profile command and can be viewed with chrome://tracing in the chrome browser.
The action queue gets filled with FPGA jobs while verilator is building the model... Verilator jobs then get kicked off as FPGA jobs complete.

copybara-service bot pushed a commit to bazelbuild/bazel that referenced this pull request Dec 30, 2022
This recreates a [closed PR](#13996) to implement extra resources which we're hoping to use in lowRISC/opentitan#16436

Fixes:#16817

Closes #16785.

PiperOrigin-RevId: 498557024
Change-Id: I60d8f8f4a4a02748147cabb4cd60a2a9b95a2c68
drewmacrae pushed a commit to drewmacrae/bazel that referenced this pull request Jan 17, 2023
This recreates a [closed PR](bazelbuild#13996) to implement extra resources which we're hoping to use in lowRISC/opentitan#16436

Fixes:bazelbuild#16817

Closes bazelbuild#16785.

PiperOrigin-RevId: 498557024
Change-Id: I60d8f8f4a4a02748147cabb4cd60a2a9b95a2c68
ShreeM01 added a commit to bazelbuild/bazel that referenced this pull request Jan 19, 2023
This recreates a [closed PR](#13996) to implement extra resources which we're hoping to use in lowRISC/opentitan#16436

Fixes:#16817

Closes #16785.

PiperOrigin-RevId: 498557024
Change-Id: I60d8f8f4a4a02748147cabb4cd60a2a9b95a2c68

Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
hvadehra pushed a commit to bazelbuild/bazel that referenced this pull request Feb 14, 2023
This recreates a [closed PR](#13996) to implement extra resources which we're hoping to use in lowRISC/opentitan#16436

Fixes:#16817

Closes #16785.

PiperOrigin-RevId: 498557024
Change-Id: I60d8f8f4a4a02748147cabb4cd60a2a9b95a2c68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant