Basic arch path - #1
Closed
bitglitcher wants to merge 31 commits into
Closed
Conversation
siddhpant
requested changes
Jul 9, 2026
| logic addr_valid; | ||
|
|
||
| assign word_addr = addr_i[ADDR_WIDTH+1:2]; // Byte to word address | ||
| assign addr_valid = (addr_i[31:ADDR_WIDTH+2] == 0); // Check upper bits are 0 |
Member
There was a problem hiding this comment.
Alignment check missing: addr_i[1:0] == 2'b00
| wire [11:0] imm = IR[31:20]; | ||
|
|
||
| wire [6:0] imm_11_5 = IR[31:25]; | ||
| wire [4:0] imm_4_0 = IR[11:7]; |
Member
There was a problem hiding this comment.
Same as rd. Let's make naming more explicit about instr type.
Member
There was a problem hiding this comment.
Actually should we even be concerned about decoding here instead of the decoder module?
|
|
||
| typedef enum logic [0:1] { FETCH, EXECUTE } exec_state; | ||
|
|
||
| exec_state current_state = FETCH; |
Member
There was a problem hiding this comment.
lets also define PC while at it
Fixes real bugs found in the reused decoder/alu/register_file modules (SLT/SRA signedness, unmasked shift amounts, inverted regfile write-enable, x0 never hardwired, missing sign-extension on every immediate type including a 12-vs-13-bit zero-pad bug specific to S-type offsets, SLLI/SRLI/SRAI reading garbage shift amounts) and widens them to WORD_SIZE=64. Adds the RV64I-only instructions (LWU/LD/SD, the *W/*IW word-arithmetic family) and two new purpose-built memory modules (imem/dmem) with combinational reads and real byte-enable writes, since the existing Wishbone-attached wb4_sram.sv is registered and can't support single-cycle timing. design/core.sv is a full rewrite: a genuine single-cycle datapath (no FSM) wiring fetch/decode/execute/memory/writeback together in one clock edge-to-edge cycle. Verified with 7 testbenches (44 checks total, all passing under iverilog, zero warnings under verilator --lint-only -Wall) targeting the specific regressions above rather than just "does it run" -- negative immediates, the SLT/SRA fixes, the S-type offset bug via an independent load-path cross-check, JALR's LSB-clear, and ADDW/SRAW vs their 64-bit equivalents on identical inputs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implement single-cycle RV64I core
Matches the SoC's 64-bit Wishbone bus so LD/SD complete as single-beat accesses; adds sel_i-gated per-lane writes for narrower stores, and a simulation-only zero-fill so uninitialized reads show 0 instead of X.
Covers a full-word round trip, a byte-enable write leaving other lanes on the same line untouched, address-line isolation, and out-of-range address asserting err_o.
Minimal transmit-only peripheral for this milestone: TX_DATA (0x8000) prints via $write on write, TX_STATUS (0x8008) is hardwired ready. Registered 1-wait-state ack, matching wb4_sram's bus timing so the CPU doesn't need to know which slave it's talking to.
Drives the Wishbone port directly and checks tx_history[]/ tx_history_count against a two-character write sequence, plus sel-gating and TX_STATUS/TX_DATA reads.
Routes the CPU's single master port to wb4_sram or uart_tx by addr_i[15]. Both slaves are registered (1-wait-state), so which slave an in-flight request belongs to is latched at issue time rather than re-derived from addr_i when the delayed ack/data shows up.
Drives the CPU-facing port against two modeled 1-wait-state slaves with distinguishable fake responses, checking routing in both directions and that the slave-select latch updates correctly across back-to-back RAM/UART transactions.
Replaces the single-cycle datapath's private combinational imem/dmem with a 3-state FSM (fetch/exec/mem) that masters the shared Wishbone bus. decoder/alu/register_file are unchanged; only the sequencing is new: commit_now gates register writes and pc updates to the one edge each instruction actually retires on, and pc[2] selects which half of a 64-bit fetch line is the 32-bit instruction actually being executed. Fixes a duplicate-transaction bug found while bringing this up: cyc_o/ stb_o were tied to the registered FSM state, which drops one cycle later than a registered slave's ack becomes visible -- long enough for a slave to see cyc/stb still asserted and service the same request twice (caught via the UART printing "HH" for a single-byte write). Fixed by gating cyc_o/stb_o with !wb_ack_i so they drop combinationally the instant ack is observed.
Runs a hand-assembled program through the real core against real wb4_sram/uart_tx/wb_addr_decoder instances (not core_alu_ops_tb.sv and friends' private imem0 -- those no longer even compile against this core.sv). Proves multi-cycle fetch, a taken branch that actually skips, a RAM load/store round trip over the bus, and a UART write all work end-to-end through the address decoder.
Connects core's Wishbone master port to wb_addr_decoder, wb4_sram, and uart_tx -- the same wiring already proven in core_wb_tb.sv, promoted to a real module. No new logic, only connections. wb4_sram is instantiated at its default 4096-word (32KB) size, which is what wb_addr_decoder's addr_i[15] address split is derived from.
Instantiates soc directly and relies entirely on wb4_sram's own $readmemh of firmware/crt0.hex -- i.e. this exercises the actual toolchain-built firmware image, not a synthetic one. Checks the UART's captured tx_history against the expected "Hello, World!\n" bytes. Must be run from one level below the repo root so the hardcoded "../firmware/crt0.hex" path resolves.
Writes each byte of "Hello, World!\n" to the UART's TX_DATA register (0x8000) and halts with ebreak. Uses a flat, repeated li+sb sequence rather than a string constant walked in a loop, since this milestone links with a bare -Ttext=0x0 and no linker script, so there's no established convention yet for where a .data section would land.
--verilog-data-width=8 matches wb4_sram's memory[] element width (was 4, sized for the old 32-bit memory). --reverse-bytes=8 corrects objcopy's byte order: its verilog writer packs address-ascending bytes as the most-significant part of each hex token, which is backwards from the little-endian memory[i][7:0]==byte@lowest_address convention $readmemh needs here.
Two issues found by actually running the build for the first time: - objcopy's byte-reversal choked on .riscv.attributes (26 bytes, not a multiple of 8) when applied to the whole ELF. Added -j .text so only the section we actually want loaded into memory gets converted. - More importantly, --reverse-bytes=8 itself was wrong. Verified by hand against objdump -d output: objcopy's verilog writer already emits bytes in the order this design's memory[i][7:0]==byte@lowest_ address convention needs, with no extra flag. The previous commit's --reverse-bytes=8 was based on an untested assumption and was actually producing the wrong byte order; removed it. End to end result: soc_tb.sv now loads the real toolchain-built firmware and observes "Hello, World!\n" over the UART, 15/15 checks passing.
Instructions now go into a real wb4_sram instance (packed two per 64-bit word, matching core.sv's pc[2] half-select) instead of the old private imem0, which no longer exists. Also widens the halted-wait timeout from 50 to 150 cycles -- multi-cycle fetch/mem phases cost more edges per instruction than the single-cycle version did.
Same change as core_alu_ops_tb: instructions poked into a real wb4_sram instead of the now-gone imem0, timeout widened to 150 cycles.
Instructions poked into a real wb4_sram instead of the now-gone imem0/dmem0. This one's timeout increase is load-bearing, not just margin: 7 of its 12 instructions are loads/stores, which now cost 5 edges each instead of 1, and the inherited timeout of 50 was exactly at that budget with zero slack -- it timed out before the fix.
Instructions poked into a real wb4_sram instead of the now-gone imem0, timeout widened to 150 cycles. Caught a transcription bug in the repacking itself while verifying: memory[7] initially had idx14 and idx15 swapped into the wrong halves AND idx15's encoding wrong (accidentally duplicated idx13's addi x7,x0,222 instead of encoding idx15's addi x7,x0,111). Net effect was a stray jal landing straight on the "skipped by jal" sentinel instead of skipping it -- caught by x8/x9 failing their checks, fixed by correctly re-deriving the pairing by hand (word = idx/2, lower half = even idx, upper half = odd idx) instead of trusting the first pass.
Same change as core_alu_ops_tb: instructions poked into a real wb4_sram instead of the now-gone imem0, timeout widened to 150 cycles.
RAM spans 0x0000-0x7FFF, matching wb_addr_decoder.sv's addr[15] split; .text is placed first so it lands at 0x0 for core.sv's reset vector (needs crt0.o to be the first object passed to ld -- this script alone doesn't guarantee that). Also defines _bss_start/_bss_end/_stack_top for crt0.s to consume. Replaces the previous bare -Ttext=0x0 link, which had no .data/.rodata/.bss/stack story at all.
Sets up sp (top of RAM, per the new link.ld) and zero-fills .bss, then calls main() -- the two things any nontrivial C function needs before it's safe to call into C at all. gp is deliberately left uninitialized; see firmware/Makefile's -mno-relax for why that's safe here. The actual hello-world logic moves to firmware/hello.c. Previously this file wrote each UART byte directly since there was no linker script to trust; that's no longer necessary now that one exists and is verified.
Freestanding (no libc): writes each byte of "Hello, World!\n" through a volatile pointer to the UART's TX_DATA register at 0x8000. volatile is load-bearing -- without it the compiler could treat the repeated writes as dead stores and drop all but the last one. Compiled at -O0 deliberately: reloads everything from the stack on every loop iteration instead of keeping values in registers, which makes this a real stress test of crt0.s's stack setup rather than something an optimizer could reduce to barely touching the stack at all.
Adds a gcc compile step for hello.c and links it with crt0.o against the new link.ld (replacing the old bare -Ttext=0x0 link). Also now converts .rodata and .data, not just .text -- hello.c's string constant lives in .rodata and needs to actually reach memory. -mno-relax on both the assembler and gcc invocations, consistently -- see crt0.s's header for why that removes the need to set up gp.
300 cycles was sized for the old hand-written assembly (2 instructions per UART byte). The -O0 C build reloads everything from the stack on every loop iteration -- around 7 memory ops per byte, each costing 5 bus edges -- and legitimately needs closer to 9500 cycles to finish; caught as a mid-run timeout (printed "Hell" then stalled), not a functional bug.
ranaumarnadeem
added a commit
to ranaumarnadeem/core
that referenced
this pull request
Aug 5, 2026
Loads the real toolchain-built csr_test.hex into a live wb4_sram instance (overwriting crt0.hex's contents after wb4_sram's own time-0 init, same quantiumv#1-delay pattern as every other core-level testbench here) and runs it through the actual core -- this is the strongest verification in the Zicsr suite, since none of it depends on this project's own hand-written encoder or hand-assembled instruction streams. 13/13 passing, independently re-derived by hand afterward with zero discrepancies (see commit history for that cross-check). s1-s11 map to gp_registers[9] and gp_registers[18..27] per the RV64 calling convention -- not a contiguous range, since a0-a7 (x10-x17) sit between s1 and s2.
Minimal Wishbone SoC: RAM + UART, hello world over iverilog
ranaumarnadeem
added a commit
to ranaumarnadeem/core
that referenced
this pull request
Aug 6, 2026
Loads the real toolchain-built csr_test.hex into a live wb4_sram instance (overwriting crt0.hex's contents after wb4_sram's own time-0 init, same quantiumv#1-delay pattern as every other core-level testbench here) and runs it through the actual core -- this is the strongest verification in the Zicsr suite, since none of it depends on this project's own hand-written encoder or hand-assembled instruction streams. 13/13 passing, independently re-derived by hand afterward with zero discrepancies. s1-s11 map to gp_registers[9] and gp_registers[18..27] per the RV64 calling convention -- not a contiguous range, since a0-a7 (x10-x17) sit between s1 and s2.
ranaumarnadeem
added a commit
to ranaumarnadeem/core
that referenced
this pull request
Aug 6, 2026
Generalizes the 3 duplicated wb_cycle task copies (uart_tx_tb.sv, wb4_sram_tb.sv, wb_addr_decoder_tb.sv) into one include-able task, preserving the load-bearing quantiumv#1-after-posedge settle timing that fixed a real duplicated-UART-character bug during uart_tx_tb.sv development. Loops on !ack && !err rather than just !ack, generalizing past the original 3 copies (which would hang forever on an err response) -- wb4_sram_tb.sv's own out-of-range test already had to hand-write this exact fix inline. CSR-port driving is deliberately not folded in here: different protocol, only 2 current consumers.
ranaumarnadeem
added a commit
that referenced
this pull request
Aug 7, 2026
Loads the real toolchain-built csr_test.hex into a live wb4_sram instance (overwriting crt0.hex's contents after wb4_sram's own time-0 init, same #1-delay pattern as every other core-level testbench here) and runs it through the actual core -- this is the strongest verification in the Zicsr suite, since none of it depends on this project's own hand-written encoder or hand-assembled instruction streams. 13/13 passing, independently re-derived by hand afterward with zero discrepancies. s1-s11 map to gp_registers[9] and gp_registers[18..27] per the RV64 calling convention -- not a contiguous range, since a0-a7 (x10-x17) sit between s1 and s2.
ranaumarnadeem
added a commit
that referenced
this pull request
Aug 7, 2026
Generalizes the 3 duplicated wb_cycle task copies (uart_tx_tb.sv, wb4_sram_tb.sv, wb_addr_decoder_tb.sv) into one include-able task, preserving the load-bearing #1-after-posedge settle timing that fixed a real duplicated-UART-character bug during uart_tx_tb.sv development. Loops on !ack && !err rather than just !ack, generalizing past the original 3 copies (which would hang forever on an err response) -- wb4_sram_tb.sv's own out-of-range test already had to hand-write this exact fix inline. CSR-port driving is deliberately not folded in here: different protocol, only 2 current consumers.
ranaumarnadeem
added a commit
that referenced
this pull request
Aug 12, 2026
…branch/jump scope gap Ran all 56 generated isa=rv64i checks for the first time: 35 passed, 21 failed. Root-caused every failure via native witness replay. Branch/jump (8 checks): riscv-formal's spec models require strict 4-byte target alignment unless RISCV_FORMAL_COMPRESSED is defined. This core implements Zca unconditionally (IALIGN=16), so the RTL was correct and the check was too strict. Fixed with one checks.cfg define, verified via source read to be scoped to exactly those 8 models. Load/store (11 checks) -- two real core.sv bugs, both fixed: 1. core.sv never checked data-access alignment at all. mem_sel's shift had no carry into a second bus word, so a misaligned load/store silently truncated and committed corrupted data instead of trapping, which the spec disallows. Added a real misalignment trap (mcause 4/6, mtval = faulting address), gating mem_phase_needed. New coverage in testbench/core_misaligned_trap_tb.sv. 2. The ifdef RISCV_FORMAL RVFI memory tap mixed two incompatible addressing conventions (exact vs. aligned address). Fixed alongside a RISCV_FORMAL_ALIGNED_MEM checks.cfg define. Two second-order bugs surfaced while implementing fix #1, caught by the existing simulation regression rather than by riscv-formal: the misalignment check initially misfired during an AMO's S_AMO_WRITE phase (mem_paddr is repurposed for the modify value there), and the RVFI address tap initially truncated to 32 bits while the spec model computes a full 64-bit expected address from the solver's free rs1_rdata. Both fixed; full 38-testbench regression and verilator lint stay clean. 54/56 checks now pass. Remaining two (ill_ch0, reg_ch0) are open, understood formal-harness/solver-performance issues, not RTL bugs -- see verification/riscv-formal/quantiumv/README.md.
bitglitcher
pushed a commit
that referenced
this pull request
Aug 16, 2026
Loads the real toolchain-built csr_test.hex into a live wb4_sram instance (overwriting crt0.hex's contents after wb4_sram's own time-0 init, same #1-delay pattern as every other core-level testbench here) and runs it through the actual core -- this is the strongest verification in the Zicsr suite, since none of it depends on this project's own hand-written encoder or hand-assembled instruction streams. 13/13 passing, independently re-derived by hand afterward with zero discrepancies. s1-s11 map to gp_registers[9] and gp_registers[18..27] per the RV64 calling convention -- not a contiguous range, since a0-a7 (x10-x17) sit between s1 and s2.
bitglitcher
pushed a commit
that referenced
this pull request
Aug 16, 2026
Generalizes the 3 duplicated wb_cycle task copies (uart_tx_tb.sv, wb4_sram_tb.sv, wb_addr_decoder_tb.sv) into one include-able task, preserving the load-bearing #1-after-posedge settle timing that fixed a real duplicated-UART-character bug during uart_tx_tb.sv development. Loops on !ack && !err rather than just !ack, generalizing past the original 3 copies (which would hang forever on an err response) -- wb4_sram_tb.sv's own out-of-range test already had to hand-write this exact fix inline. CSR-port driving is deliberately not folded in here: different protocol, only 2 current consumers.
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.
Add supporting file for the 2 cycle RV32I Core.
There was a change of plans. After a quick discussion with team members.
We have decided to change course a simple RV32I architecture.