Default picosoc register file doesn't support q regs

[gundy]

Hi.

Today I discovered that the default PicoSoc configuration doesn't seem to support the q registers, which are used for saving registers in IRQ handlers.

The test case that I used was a basic IRQ handler in ASM. In my test case, this worked fine:

irq_vec:
    picorv32_retirq_insn()

.. but this didn’t:

irq_vec:
    // backup x10/x11 in q2/3
    picorv32_setq_insn(q2, x10)
    picorv32_setq_insn(q3, x11)

    // modify X10/X11
    addi x10, zero, 0
    addi x11, zero, 0

    // restore x10 and x11 from Q registers
    picorv32_getq_insn(x10, q2)
    picorv32_getq_insn(x11, q3)

    // return from IRQ
    picorv32_retirq_insn()

It turned out that the issue was in the default register bank provided by PicoSoc, and the fact that picorv32 uses registers r32-r35 for the Q registers. The default register bank truncates addresses to 5 bits, so the q registers don't work.

module picosoc_regs (
    input clk, wen,
    input [5:0] waddr,
    input [5:0] raddr1,    
    input [5:0] raddr2,
    input [31:0] wdata,
    output [31:0] rdata1,
    output [31:0] rdata2
);

reg [31:0] regs [0:31];
always @(posedge clk)
    if (wen) regs[waddr[4:0]] <= wdata;  // <---- address truncated

    assign rdata1 = regs[raddr1[4:0]];   // <---- address truncated
    assign rdata2 = regs[raddr2[4:0]];   // <---- address truncated
endmodule

I've got a pull request ready to go - I'll send it through for review shortly.

D.

[gundy]

I've submitted pull request #87 to address this.

[cliffordwolf]

This is intentional and matches the default configuration of PicoRV32. Otherwise the regs file would be twice as large as needed in the default configuration. picorv32_regs is just a simple example. If you set PICORV32_REGS then you would of course provide your own implementation that has the size you need.

The comment on picorv32_regs states all that, even explicitly reminds you that you must make appropriate changes if you want to use it with QREGS:

// This is a simple example implementation of PICORV32_REGS.
// Use the PICORV32_REGS mechanism if you want to use custom
// memory resources to implement the processor register file.
// Note that your implementation must match the requirements of
// the PicoRV32 configuration. (e.g. QREGS, etc)
module picorv32_regs ...
endmodule

[gundy]

👍 Fair enough - sorry about that..

My default configuration came from the TinyFPGA-BX repo's picosoc, where QREGS was enabled and the reg file still only had 32-registers .. Consequently I just spent the better part of a day chasing this one down. I somehow missed that the defaults here were different.

Sorry for wasting your time with this.

D.