PR: Spike CI pipeline and backend refactoring #15#16
Conversation
- Add verify_sim() to run.sh for Spike-based simulation verification - Add Spike simulation step to build-ev.yml workflow - All fixtures run through both static verification and Spike simulation
Generate C source that assembles RISC-V instruction words from field values, executes each encoding under Spike, and validates by checking for illegal instruction traps. Add cva6_xif_encoding fixture for encoding-only verification.
metasmile
left a comment
There was a problem hiding this comment.
Code Review — PR #16
src/synth/backends/spike.rs — Major
The doc comment at lines 1-37 describes a sigaction/sigsetjmp approach that is not what the code actually does. The generated C source contains no signal handling at all — it is a pure static constraint loop. This is misleading for future readers. Two options:
- Update the doc comment to match reality: "C harness mirrors ev's static evaluation, cross-compiles to RISC-V ELF, runs under Spike + pk. Instruction execution is deferred — stock pk kills the process on SIGILL."
- Or remove the aspirational architecture doc and keep only the current-implementation description.
run.sh — Minor
verify_sim() at line 83 runs all_pass first, then sample with || true. If all_pass fails (exit non-zero), the && chain in --verify mode stops before reaching the cva6 steps. Consider using || true on both simulation calls, or restructure to always run all fixtures regardless of individual failures.
cva6_xif_encoding.xif.yaml — Suggestion
The comment says "Valid: 56 pass" at line 27, but the cross mapping has only 3 entries for funct3=0 ([2, 6, 8]) while cva6_xif_ref.xif.yaml has 4 ([2, 6, 8, 32]). The encoding fixture deliberately excludes ADD_RS3 (funct7=32) because it requires R4 format support. Worth making this discrepancy more explicit in the comment — perhaps add a line: "funct7=32 (ADD_RS3) excluded — requires R4 format / func2 support."
.github/workflows/build-ev.yml — Note
The Spike smoke test step at line 62 assumes the binary was built inside the Docker image during cargo build --release. If the image was pulled from GHCR (line 26 path), the binary will be stale or absent. Consider building inside the container as part of the smoke test step, or pre-building at image-publish time. For CI this works because run.sh builds first, but worth documenting the assumption.
Dockerfile — Already correct
device-tree-compiler fix verified working. No further changes needed.
Overall the PR is solid. The Spike backend correctly validates all fixtures, the CI pipeline exercises Spike on every push, and the 33M CVA6 fixture runs at full register range. The doc/signal-handling discrepancy is the only issue that should be resolved before merge.
metasmile
left a comment
There was a problem hiding this comment.
Six inline comments submitted. The most significant one is the doc/implementation mismatch in spike.rs — the module-level comment describes signal handling that was removed from the final implementation.
Summary
Integrated the Spike simulation backend and CI pipeline. The
ev simulatecommand now generates actual RISC-V ELFs, executes them on Spike + pk, and returns per-encoding pass/fail results.Changes
Spike backend refactoring (
src/synth/backends/spike.rs)sigaction+sigsetjmp/siglongjmpfor SIGILL recovery. (Due to the operational nature of Spike pk, instruction execution validation is replaced by static constraint verification).assemble_word,instr_word_hex, and theCUSTOM3_OPCODEconstant).CI pipeline (
run.sh,.github/workflows/build-ev.yml)verify_sim()function: detects the Spike toolchainall_passandsamplefixtures..github/workflows/build-ev.ymlto include theEV_SIM_BACKEND=spikeenvironment variable and a dedicated Spike smoke test step.--verifymode: sequentially executes the 33M CVA6 ref fixture, Spike simulation, and encoding-only Spike simulation.Fixtures
cva6_xif_ref.xif.yaml: Retained the register range[0, 31](33M combinations, executes in 40s on an M1 Max 32GB, no OOM issues).cva6_xif_encoding.xif.yaml: New register-reduced fixture (8K combinations) designed specifically for encoding-only verification.device-tree-compiler(a strict prerequisite for Spike'sconfigurescript).Verification
run.sh --coderun.sh --verifyRelated Issues