Skip to content

Add --config-override flag for configuration overrides#1534

Merged
pmundkur merged 1 commit intoriscv:masterfrom
trdthg:first-party-test
Feb 12, 2026
Merged

Add --config-override flag for configuration overrides#1534
pmundkur merged 1 commit intoriscv:masterfrom
trdthg:first-party-test

Conversation

@trdthg
Copy link
Collaborator

@trdthg trdthg commented Feb 6, 2026

This adds a --config-override option to the simulator to override parts of the base --config (or the built-in default). This is especially useful for first-party tests, some of which need the config to be slightly tweaked, for example Zclsd and Zcf are exclusive.

Fixes #806

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Test Results

2 328 tests  ±0   2 328 ✅ ±0   31m 8s ⏱️ -11s
    1 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 961ec5e. ± Comparison against base commit 5f5f02d.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@pmundkur pmundkur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems useful! Some more documentation would be nice.

@trdthg trdthg force-pushed the first-party-test branch 4 times, most recently from 464ad50 to 395eb0a Compare February 9, 2026 09:13
Copy link
Collaborator

@Timmmm Timmmm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good to me. I'm still not sure what we do about error location reporting though.

@Timmmm Timmmm added the tgmm-agenda Tagged for the next Golden Model meeting agenda. label Feb 9, 2026
@Timmmm
Copy link
Collaborator

Timmmm commented Feb 9, 2026

I'm still not sure what we do about error location reporting though.

For the record, discussed in the meeting. We decided just to go with it. In theory you shouldn't ever get a parse error from Sail because we always parse with jsoncons first. That may not end up being 100% true if there are subtle differences between the parsers, or maybe things that Sail checks that the schema validation doesn't, but fingers crossed...

@trdthg
Copy link
Collaborator Author

trdthg commented Feb 10, 2026

I've made a clear distinction between parse and validate errors.

The code first run json::parse on every input config file, and then outputs the filename and location info for any caught errors.

$ ./build/c_emulator/sail_riscv_sim --config ./rv32d_v128_e32.json --config-override ./test_A.json test.elf
JSON parse error in override file ./test_A.json:
Extra comma at line 4 and column 4

Now it doesn't matter whether os << config_json uses pretty_print or not, because any formatting issues will have already cause an early return.

Finally, we perform the validation check. validate_config_schema now accepts a jsoncon::json, we can guarantee it will only throw schema-related exceptions.

$ ./build/c_emulator/sail_riscv_sim --config ./rv32d_v128_e32.json --config-override ./test_A.json test.elf
Error: Schema conformance check failed for merged configuration from ./rv32d_v128_e32.json, ./test_A.json:
- /base: Required property 'xlen' not found.

@trdthg trdthg force-pushed the first-party-test branch 2 times, most recently from 82b131d to b01acea Compare February 11, 2026 01:04
Copy link
Collaborator

@Timmmm Timmmm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work!

Copy link
Collaborator

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Probably worth mentioning this in the README and definitely warrants an entry in the Changelog. The PR title should also be updated because this is not exclusive to first-party tests.

@trdthg trdthg changed the title Support custom configs for first-party tests Add --override-json flag for configuration overrides Feb 11, 2026
@Timmmm Timmmm changed the title Add --override-json flag for configuration overrides Add --config-override flag for configuration overrides Feb 11, 2026
@Timmmm
Copy link
Collaborator

Timmmm commented Feb 11, 2026

We may want to add a proper man page style document for the simulator at this point, rather than just adding everything to the readme.

Copy link
Collaborator

@pmundkur pmundkur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Minor tweaks to the Changelog and README.

Introduces a new CLI option `--config-override` that allow us to specify an
additional JSON configuration to override the corresponding fields in default
or custom configuration.

This option is also helpful for first-party tests. It allows us to specify
different configs for different tests without writing complete configuration
files.
Copy link
Collaborator

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@pmundkur pmundkur added will be merged Scheduled to be merged soon if nobody objects and removed tgmm-agenda Tagged for the next Golden Model meeting agenda. labels Feb 11, 2026
@pmundkur pmundkur added this pull request to the merge queue Feb 12, 2026
Merged via the queue into riscv:master with commit c405298 Feb 12, 2026
14 checks passed
lfiolhais added a commit to lfiolhais/sail-riscv that referenced this pull request Feb 19, 2026
* origin/master: (51 commits)
  Update version. (riscv#1552)
  Update changelog for version 0.10. (riscv#1526)
  Adapt the release workflow to use the common `sail-setup` action. (riscv#1543)
  Use FetchContent for CLI11 and bump to v2.6.1 (riscv#1544)
  Add the Rocky Linux container build to CI. (riscv#1542)
  Add missed stateen checks for the high-half CSRs of `hstateen[0-3]`. (riscv#1546)
  Add --config-override flag for configuration overrides (riscv#1534)
  Fix fcvt.s.bf16 NaN-boxing returning wrong canonical QNaN (riscv#1528)
  Improve README. (riscv#1540)
  Make global `g_model` instance local (riscv#1536)
  Install newer Clang version on Linux CI for newer extension support (riscv#1532)
  Strengthen some before statements in sail_project file (riscv#1537)
  Make `config` settings non-global (riscv#1535)
  Remove some unused string mappings. (riscv#1538)
  Return instead of exit() in `inner_main()` (riscv#1531)
  Move CLI options into struct (riscv#1529)
  Remove unncessary void parameters in riscv_sim.cpp (riscv#1530)
  Add info on contributing extensions under development. (riscv#1521)
  Remove config_enable_rvfi checks from RVFI callbacks (riscv#1525)
  Validate vector register groups. (riscv#1486)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

will be merged Scheduled to be merged soon if nobody objects

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hard-coded ISA string for building all first_party tests

4 participants