YosysHQ / picorv32

PicoRV32 - A Size-Optimized RISC-V CPU
ISC License
3.14k stars 755 forks source link

Trap in simulation with existing compiler #210

Closed pcotret closed 1 year ago

pcotret commented 2 years ago

I was trying to run make test with my existing 32-bit compiler, I got the following error:

[...]
python3 firmware/makehex.py firmware/firmware.bin 32768 > firmware/firmware.hex
vvp -N testbench.vvp
TRAP after 8224 clock cycles

My compiler was configured with:

$ ./configure --prefix=/opt/riscv_compiler --with-arch=rv32gc --with-abi=ilp32 --disable-threads

Any idea which flag is missing (or maybe several flags) ?

retrhelo commented 2 years ago

Just encounter the same problem.

The problem seems to be caused by an unalign memory access in the early part of firmware/start.S.

.balign 16
irq_vec:
    /* save registers */

#ifdef ENABLE_QREGS

    picorv32_setq_insn(q2, x1)
    picorv32_setq_insn(q3, x2)

    # Load the address of irq_regs into x1, aka sp
    lui x1, %hi(irq_regs)
    addi x1, x1, %lo(irq_regs)

    picorv32_getq_insn(x2, q0)
    sw x2,   0*4(x1) # This very instruction causes an unaligned memory access

However, the symbol irq_regs follows right behind some other instructions, and aligned only if ENABLE_QREGS is not defined (which is defined by default in the first few lines of firmware/start.S). When compiled without compressed instructions, all RISC-V instructions remain 4-byte long, thus irq_regs has an alignment of 4-byte. However, when compiling with compressed ones, it's possible for irq_regs to align to 2-byte, leading to the unaligned memory access presented above.

    lw x28, 28*4+0x200(zero)
    lw x29, 29*4+0x200(zero)
    lw x30, 30*4+0x200(zero)
    lw x31, 31*4+0x200(zero)
#endif

#endif // ENABLE_QREGS

    picorv32_retirq_insn()

#ifndef ENABLE_QREGS
.balign 0x200
#endif
irq_regs:
    // registers are saved to this memory region during interrupt handling
    // the program counter is saved as register 0
    .fill 32,4

    // stack for the interrupt handler
    .fill 128,4
irq_stack:

I have no idea why irq_regs isn't always set to be aligned. But to solve the problem in this issue, we can simply remove the if-block around .balign to make sure that irq_regs is always aligned.

AshvinVaidyanathan commented 1 year ago

I tried removing the ifndef arounf .balign 0x200 but still doesn't solve the issue. Also tried the solutions Patrick @patrickerich mentioned. The make test still throws a

mismatch between q0 LSB and decoded instruction word! q0=0x0000BE78, instr=0x8286
TRAP after 86618 clock cycles
ERROR!
patrickerich commented 1 year ago

I haven't looked at this core for a while, but for me it seems that the problem was related to compiling the RISCV toolchain from source (although I am not sure about the exact cause).

When using a pre-compiled toolchain the make test seems to run without problems. I just tried it (succesfully) with the precompiled toolchain from lowrisc, which you can get here

If you want to try it, simply: 1) download the lowrisc-toolchain-rv32imcb-20220524-1.tar.xz 2) unpack it somewhere (e.g. /opt/lowrisc) 3) modify the TOOLCHAIN_PREFIX variable in the Makefile (e.g. to TOOLCHAIN_PREFIX = /opt/lowrisc/lowrisc-toolchain-rv32imcb-20220524-1/bin/riscv32-unknown-elf-

HTH

AshvinVaidyanathan commented 1 year ago

Hi @patrickerich !

Thanks for the quick reply. i did try the pre-compiled toolchain but it still doesn't work. I made sure to clean the existing files before running it with the new toolchain as well. Still getting the same error.

patrickerich commented 1 year ago

Hi @AshvinVaidyanathan,

Short reply: Make sure that COMPRESSED_ISA = C in your Makefile!

Longer reply: It is probably a good idea to checkout a fresh copy of the repo (to start with a 'clean' Makefile). I suspect that my previous comment could be (part of) the cause of your current problem. Please note that I removed my previous comment (as shown below) to prevent it from causing any other problems.

I ran into the same TRAP (same number of clock cycles) and it seems to be related to whether or not compressed instructions are used. I just started with this core (so I am not yet very familiar with it), but by changing (in the Makefile): COMPRESSED_ISA = C to: COMPRESSED_ISA = I was able to run the make test

When I check out a fresh copy of the git repo and ONLY change the TOOLCHAIN_PREFIX variable (in the Makefile) to point to a precompiled toolchain, then make test runs without any problems. If additionally I change COMPRESSED_ISA = C to: COMPRESSED_ISA =, I get an error that is almost identical to yours:

 Mismatch between q0 LSB and decoded instruction word! q0=0x0000BCAC, instr=0x8286
TRAP after 86613 clock cycles
ERROR!

HTH

patrickerich commented 1 year ago

Hi @AshvinVaidyanathan,

The solution posted by @retrhelo is completely correct (Thanks @retrhelo!). I tried to do a fresh compile of the riscv-gnu-toolchain (eventually even a multilib build) and the solution posted by @retrhelo does indeed solve the issue first mentioned by @pcotret.

I suspect that the reason that the lowrisc pre-build toolchain did not show the same problem (for me), is because of the configuration of the build. I am not sure exactly which option made the difference, but I did notice that a --disable-libmpx flag was used in their build.

AshvinVaidyanathan commented 1 year ago

Hi @patrickerich,

Thanks for your patience. A clean reinstall did the trick.