Conversation
run_rtl.py reads 'sim_opts' out of the test options for both riscv-dv and directed tests, but the pydantic schema that validates a directed test's configuration has no such field, so pydantic's default extra="ignore" dropped it. A directed test therefore could not pass a plusarg through its own testlist entry. Add the field to DConfig, optional so every existing entry stays valid, and on DConfig rather than DTest so a config can supply a default that an individual test still overrides after the merge. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
The append that accepts a test sat inside the loop over its rtl_params, so a test listing N parameters was scheduled N times: make warned that its targets were given more than once and regr.log counted every seed N times. Every existing entry lists a single parameter, which hid it. Append in the loop's else clause instead, once all parameters matched. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
gen_testlist.py enumerated the vendored test directories with ls, whose order follows the collation of the current locale: under C/POSIX sh-misaligned sorts before shamt, under en_US after it. Regenerating directed_testlist.yaml therefore produced a different file depending on the environment, and the committed file could not be checked against its generator. List the directories with os.listdir() and sort the names in code point order, which does not depend on the locale. Regenerate directed_testlist.yaml: no entry changes, only the order of the vendored tests. Signed-off-by: Kulan Palanichamy <kulan.palanichamy@opentitan.org>
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document. By submitting this pull request comment, I am hereby confirming my acceptance of the terms of the CLA Document and my agreement to be legally bound by its terms. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three small fixes to the directed-test flow in
dv/uvm/core_ibex. None changes whichtests run on the baseline; they are prerequisites for directed tests that need a
plusarg of their own (Zcmp and dummy-instruction tests in follow-up PRs, see #2461).
sim_optsis silently dropped from directed-test entriesscripts/run_rtl.pyreadssim_optsfrom the test options for both riscv-dv anddirected tests, but the pydantic schema for
directed_testlist.yaml(
scripts/directed_test_schema.py) has no such field, so pydantic's defaultextra = "ignore"discarded it. A directed test could not pass a plusarg through itsown entry. The field is added to
DConfigasOptional[str] = None: existingentries stay valid, and a config default can still be overridden per test.
A test with N
rtl_paramsentries is scheduled N timesIn
scripts/ibex_cmd.py::filter_tests_by_configthe acceptingappendsat insidethe loop over
rtl_params, so a test listing N parameters was appended N times:makewarned about duplicate targets andregr.logcounted every seed N times.Every existing entry lists one parameter, which hid it. The append moves to the
loop's
else:clause, which runs once and only when no parameter caused abreak.directed_testlist.yamlregenerates differently per localedirected_tests/gen_testlist.pylisted the vendored test directories withls,whose order follows the locale's collation, so the committed YAML could not be
checked against its generator. The listings now come from
sorted(os.listdir()),which is code-point order on every machine. The regenerated YAML is included.
Verification
Baseline
34b07057.contents are unchanged; only the order of a few vendored tests differs. With the
unmodified generator, running under the C locale produces a different file from the
committed one.
filter_tests_by_configon theopentitanconfig returns a test withtwo
rtl_paramsentries twice before this change and once after; one-parameter,no-parameter and rejected tests behave the same before and after.
ibex_cosim,IBEX_CONFIG=opentitan): adirected test with two
rtl_paramsentries andsim_opts: +disable_cosim=1isscheduled once and the plusarg appears on the simulator command line. Existing
directed tests and
riscv_arithmetic_basic_testpass unchanged.util/lint_commits.py --no-mergesclean.Disclosure
Per CLA section 9: the analysis and code in this PR were produced with the
assistance of an AI coding tool (Claude Code) and independently reviewed with a
second tool (OpenAI Codex). I have reviewed and understood every change and take
full responsibility for it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rn6C7Eyq1LchFeP9w6up9P