[bazel] Pass copt and features through the OT transition
#20388
Conversation
cfrantz
force-pushed
the
bazel-rules
branch
3 times, most recently
from
cc37e04 to
acb6382
Compare
|
Shouldn't we keep the transition-supplied configuration separate from the target platform's? In the case where the host is the target platform, there may be parameters that only apply to the host software and actually cause errors for the opentitan platform. Perhaps some build settings could be used to supply those instead? The compilation_mode is one part that has given me trouble in the past, for example. If I just want to debug host tools, I sometimes can't use the |
This is specifically to be able to build englishbreakfast programs using the new rules. The opentitan plaform transition is discarding the This change is concerned with getting the englishbreakfast programs to build with the new rules, thus allowing us to eliminate any of the old |
cfrantz
force-pushed
the
bazel-rules
branch
5 times, most recently
from
afa4f4b to
7ec84f4
Compare
azure-pipelines.yml
Outdated
| - bash: | | ||
| . util/build_consts.sh | ||
| mkdir -p "$BIN_DIR/hw/top_earlgrey/" | ||
| cp $(ci/scripts/target-location.sh //hw:verilator) \ | ||
| cp $(./bazelisk.sh outquery-build.verilator //sw/device/tests:uart_smoketest_sim_verilator) \ | ||
| "$BIN_DIR/hw/top_earlgrey/Vchip_earlgrey_verilator" | ||
| displayName: Copy //hw:verilator to $BIN_DIR |
There was a problem hiding this comment.
Change the displayName to reflect line 350?
There was a problem hiding this comment.
It still is //hw:verilator, but indirectly by way of a test rule :)
It has to be done that way because //hw:verilator is built under the platform transition of the test rule (it transitions back to the host, but that causes the somewhat confusing bindir name of k8-opt-exec-${host}-ST-${target}.
As a compromise, I've changed the text to read "Copy verilated binary to $BIN_DIR".
| @@ -1220,14 +1220,14 @@ | |||
| { | |||
| name: chip_sw_aes_enc | |||
| uvm_test_seq: chip_sw_base_vseq | |||
| sw_images: ["//sw/device/tests:aes_smoketest:1"] | |||
| sw_images: ["//sw/device/tests:aes_smoketest:1:new_rules"] | |||
There was a problem hiding this comment.
By the way, it seems there are some other "sw_images" missing the new_rules suffix.
There was a problem hiding this comment.
Can you point out where I've missed them? I can't seem to find any with grep.
| '--features=-rv32_bitmanip', | ||
| '--copt=-DOT_IS_ENGLISH_BREAKFAST_REDUCED_SUPPORT_FOR_INTERNAL_USE_ONLY_', |
There was a problem hiding this comment.
And these are the --features and --copt you intended to pass for the risc V code generation. Along the line of @a-will's concern, is it safe to assume no other --copt flags will sneak in via some other kind of command? For example, the regular test rules will build both harness and device code, so if --copt was given in the bazel test invocation intended for the harness it may sneak through.
There was a problem hiding this comment.
The --copt and --features serve two purposes:
- The copts identify the platform as englishbreakfast and cause the preprocessor symbol
OT_IS_ENGLISH_BREAKFASTto get defined (inmacros.h). - The features identify CPU code generation options for the specific rv32imc implementation in englishbreakfast. Most notably, it disables the bitmanip and crc32 extensions.
Unfortunately, the copts currently have to be passed through to host code generation to make sure the correct enumerations/defines are available for DIFs (e.g. rstmgr reasons) and for bindgen'ing the constants in to Rust opentitanlib/tool.
The feature flags matter less, but they also have no meaning to the host compiler - the bazel compiler configuration for the host doesn't have a feature called rv32_bitmanip and ignores it.
I don't particularly like this situation with copt leaking into other configurations, but until we change the overall hacky way englishbreakfast is handled in the codebase, we need it. Eliminating all legacy usage of opentitan_functest in favor of the new rules will help us get to a configuration where englishbreakfast is a true separate top supported by exec_envs rather than by the current tree-dirtying operation.
1. Pass the `copt` and `features` values through the OT platform transition. 2. Convert the aes_smoketest to the `opentitan_test` rule. 3. Refine the verilator binary query for the englishbreakfast model. Signed-off-by: Chris Frantz <[email protected]> Signed-off-by: Chris Frantz <[email protected]>
|
This incidentally seems to have reduced Fast Earl Grey Verilated Tests runtime by ~10 minutes in CI. Thanks! |
coptandfeaturesvalues through the OT platform transition.opentitan_testrule.