Rv64 m3 - #41
Merged
Merged
Conversation
… pc_reg from fetch The pending-prediction consume (clear_pending_prediction_state) fired on pending_prediction_target_handoff unconditionally, while the pc_reg flop it hands off to is stall-gated. A stall landing exactly when pc_reg reaches the predicted branch PC discarded the pending target with pc_reg frozen: pc_reg then advanced SEQUENTIALLY past the predicted-taken branch while fetch followed the target, and the aligner served target-path bytes under sequential pc_reg addresses. Decode manufactures coherent-looking phantom instructions from that pairing -- a non-branch can dispatch wearing a branch's identity, "resolve" as a taken mispredict to pc+garbage-imm, train the BTB with the phantom entry, and redirect the machine mid-basic-block into code that never architecturally executed (observed as wild stores trapping with mcause=6/mtval=0xb, with silent corruption also possible). Pre-existing well before the rv64 work (reproduces identically at 9b76e39); every -O2 build dodged it by code-layout luck. Exposed by the CoreMark-PRO -O3 rebuild: genesys2 -v1 official runs of core and nnet_test trapped in the report phase after verifying clean, reproduced cycle-exactly by the coremark_pro_core cocotb sim, and root-caused by walking the retirement, BTB-update, ROB-allocation, ID-record, and pending-state traces down to the single ungated consume term. Gate the consume on !fetch_stall, matching exactly the condition under which the next_pc_reg mux can apply the target (the !i_fetch_progress hold arm sits above the handoff arms; pc_update_en blocks i_stall). The crossing arm needs no gate: it consumes implicitly via stale_pending_prediction only after pc_reg really advances past the branch. A sim-only assertion (p_handoff_consume_implies_apply) documents and polices the remaining assumption that no higher-priority non-redirect mux arm (window-cannot-serve resteer, slot-2 prediction, post-handoff holdoff) wins on an un-stalled consume cycle; an independent adversarial review of the fix argument confirmed the stall-path reasoning and named those arms as the residual proof obligations, all of which predate this change and consume identically before and after it. Verified: coremark_pro_core cocotb fails pre-fix (phantom BEQ dispatched at the c.or at 0x4134, recovery redirect to 0x4164, trap) and passes post-fix with the pending state held across the stall and applied at release; regate of coremark_pro_{core,nnet}, c_ext_test_fetch_fuzz, branch_pred_test_fetch_fuzz, and control_flow_tracker.
Switch the CoreMark-PRO port's shared optimization level from -O2 to -O3 via the repo-standard OPT_LEVEL variable (env-overridable for A/B runs, e.g. OPT_LEVEL=-O2 ./fpga/sweep_coremark_pro.py ...). -O3 is legal under the vendored run rules: "You may change toolchain and build/load/run options" (coremark-pro README, Run Rules / Allowed #2). The only flag-related requirements are identical compile+link flags across all nine workloads -- preserved by construction, since BASE_FLAGS feeds every translation unit and the link -- and disclosure alongside published scores. The -O2 in EEMBC's shipped gcc*.mak files is reference-toolchain convention, not a rule, and the bare-metal porting note (CMP01 sec. 3.6) explicitly invites tuning compiler options once baselines exist. All nine official workloads build warning-free at -O3 and fit their memory regions. This also closes the asymmetry against MicroBlaze V comparisons, whose published CoreMark-PRO binaries are built at -O3. Rewrite the two Makefile comments that cited an upstream "util/make/gccrv32.mak" toolchain file: no such file exists anywhere in the vendored tree; -O2 had been inherited convention, not a documented decision. Pre-scale the per-board -v0 iteration counts (~1.4x integer, ~1.5x FP, targeting ~10.5s) so the first -O3 score sweep clears the 10-second score-rule floor in one pass: the -O2-calibrated counts sat at 10.1-10.5s of measured runtime, so any -O3 speedup pushes them under the floor (the X3 counts additionally predate the M1 64-bit data tier's FP uplift). Counts and comments are interim pending measured -O3 times from the next hardware sweep; BASELINE_SCORES intentionally unchanged until then. The -O3 rebuild's code layout exposed the pre-existing pending-prediction handoff bug fixed in the previous commit; hardware -v1/-v0 reruns need a bitstream carrying that fix.
X3 131.22 (was 111.14 at -O2 before the 64-bit data tier: +18.1%), genesys2 45.32 (was 44.41 at -O2 after the data tier: +2.05% from -O3 alone). Full -v1 validation passes 9/9 on both boards on bitstreams carrying the pending-prediction handoff fix -- including core and nnet_test, the two workloads whose -v1 runs exposed that bug. Replace the interim pre-scaled iteration counts with values derived from the measured -O3 rates, targeting the usual ~10s-plus-margin discipline. The pre-scale survived first contact on 17 of 18 board/workload pairs; X3 radix2 outran the 1.5x FP allowance (1.69x: its DDR-resident FFT tables benefit doubly from the 64-bit data tier at 300 MHz) and fell to 9.36s, so its count rises 98 -> 110. X3 nnet's raise to 2 iterations is now measurement-backed: one iteration runs ~9.8s, under the floor. The radix2 mark sample was taken at 9.36s; the next -v0 sweep at the new count re-takes it rules-clean (scores are rate-based, so the mark moves at most noise from 131.22). Per-workload -O3-plus-data-tier gains track closely across boards (nnet +66%, linear_alg +32%, cjpeg +8.2%, sha +3.2% on both; core -2.9% on both from -O3 code growth); radix2 diverges by design (X3 +69% vs genesys2 +32%: memory-bound FFT vs core clock) and parser via its documented heap sensitivity.
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
1 participant
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.
No description provided.