feat: complete safe RV32 constant-load merge optimization - #35
Conversation
🤖 AI Code Review
📁
|
a04a5a8 to
179f885
Compare
yuki-328
changed the title
The AI code review workflow failed (exit 28) on large PRs because deepseek-v4-flash processing a big file diff (e.g. ci.yml ~14KB payload) hangs past the curl timeout, and --retry-all-errors then retried the subsequent HTTP 429 quota errors into a 180s+ hang + curl exit 28, which the implicit set -e (GitHub Actions default bash) turned into a job failure. Local repro on real PR ScratchV-Compiler#35 data (same key and workflow curl args, model swapped): - deepseek-v4-flash: first file times out (exit 28), job aborts - glm-5.2: 429 Workspace allocated quota exceeded (key has no quota) - sensenova-6.8-flash-lite: all files return 200, stable Changes: - Switch model to sensenova-6.8-flash-lite (verified working). - Drop --retry-all-errors so HTTP 429 (quota) is not retried into a hang. - Wrap curl in || { [WARN] ...; continue; } so a per-file curl failure is treated as 'no result' and skipped while the job still succeeds, instead of aborting via set -e. - Set --max-time 90 --retry 0: gives large diffs enough time while capping worst case at 90s/file (10 files fit inside timeout-minutes: 15).
…tal (#52) The AI code review workflow failed (exit 28) on large PRs because deepseek-v4-flash processing a big file diff (e.g. ci.yml ~14KB payload) hangs past the curl timeout, and --retry-all-errors then retried the subsequent HTTP 429 quota errors into a 180s+ hang + curl exit 28, which the implicit set -e (GitHub Actions default bash) turned into a job failure. Local repro on real PR #35 data (same key and workflow curl args, model swapped): - deepseek-v4-flash: first file times out (exit 28), job aborts - glm-5.2: 429 Workspace allocated quota exceeded (key has no quota) - sensenova-6.8-flash-lite: all files return 200, stable Changes: - Switch model to sensenova-6.8-flash-lite (verified working). - Drop --retry-all-errors so HTTP 429 (quota) is not retried into a hang. - Wrap curl in || { [WARN] ...; continue; } so a per-file curl failure is treated as 'no result' and skipped while the job still succeeds, instead of aborting via set -e. - Set --max-time 90 --retry 0: gives large diffs enough time while capping worst case at 90s/file (10 files fit inside timeout-minutes: 15).
2 participants
Summary
lui+addipairs and remove redundantluionly within basic blocksliencoding across the signed 32-bit rangeStatistics semantics
candidate_pairs: structural same-blocklui/addicandidates before safety checksmerged_pairs: candidates that pass register and immediate validation and are transformedredundant_lui_removed: safely removed duplicate high-immediate loadsReal benchmark zero-hit cases are retained.
liis treated as a pseudo-instruction, so source assembly reductions are not presented as machine-code or runtime wins.Deterministic case report
The benchmark job now runs
benchmarks/cases/const_merge_feature.asmthrough the configuredCompilerDriverassembly post-pass and then executes the encoded before/after programs with the real TinyFive backend.Additional report results:
compiler_config_const_merge=truefeature_used=truemerged_pairs=1redundant_lui_removed=1backend=tinyfivefallback=falsex5=4098,x6=8195,x7=7The difference between the source reduction (2) and machine-instruction reduction (1) is expected:
li x5, 4098is a pseudo-instruction and expands to a reallui/addipair. This case demonstrates feature integration and execution equivalence; it is not presented as a real-workload speedup.CI writes
benchmark_reports/const_merge_report.jsonandbenchmark_reports/const_merge_report.md, uploads them in thebenchmark-reportsartifact, and includes the Markdown result in the job summary.The older generic
tinyfive_compare.pypath still falls back for instructions unsupported by the local encoder (slli/ecall). Its output now explicitly saysstatic-analysis-fallbackand is not used as proof of TinyFive execution. The deterministic report rejects any fallback instead.CNN real-model A/B benchmark
The CI CNN step now invokes the standalone compiler with
--const-merge. It generates baseline and optimized code from identical memory plans, then reports measured encoded output sizes rather than substituting expected values.Pass statistics are
candidate_pairs=70,merged_pairs=70, andredundant_lui_removed=0. Source-level merges exceed the machine-instruction reduction because largelipseudo-instructions still encode aslui+addi; 11 merged constants fit a single encoded instruction.The 2.20B dynamic-instruction figure remains the existing analytical CNN estimate, not an end-to-end A/B simulation, so this report claims a measured static/code-size reduction rather than a fabricated runtime speedup.
Validation
pytest -q: 459 passed, 4 skipped, 0 failedonnxruntimefallback=false,output_equal=trueliboundary cases validated from-2147483648through2147483647compileall: passedgit diff --check: passedKnown environment limitation
The deterministic A/B report now provides real TinyFive execution evidence. GNU RISC-V binutils/Spike external-toolchain validation is still pending and is not represented as completed.