Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unaligned data error #25

Closed
stevobailey opened this issue Jan 23, 2022 · 6 comments
Closed

Unaligned data error #25

stevobailey opened this issue Jan 23, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@stevobailey
Copy link
Contributor

stevobailey commented Jan 23, 2022

Can Vicuna handle misaligned data? When simulating an 8-bit vector add using Spike and unaligned data, it works correctly. But when the same program is run on Vicuna RTL, it fails. It think it should work even on misaligned data.

I can recreate this by running your vector add test (alu/vadd_8.S), but adding an offset to the data:

...
    .data
    .align 10
    .global vdata_start
    .global vdata_end
    .byte           0
vdata_start:
    .word           0x323b3f47
    .word           0x47434b3a
    .word           0x302f2e32
...
[CONFIG ] alu/vadd_8 VREG_W=128  VMEM_W=32   VMUL_W=32
[ ERROR ] alu/vadd_8/vadd_8                    607 cycles (       12 -       619)
incorrect memory content; diff:
--- vadd_8.ref.vmem
+++ vadd_8.dump.vmem
@@ -1,7 +1,7 @@
 333c4048
 48444c3b
 31302f33
-e9414b52
+e8414b52
 3f44383b
 37424d54
 5e4b5049
make: *** [Makefile:64: alu/vadd_8] Error 1

@stevobailey
Copy link
Contributor Author

Perhaps to correct my nomenclature, I think an array with 8-bit data is never misaligned. Because the data are 8 bits, any starting address in bytes is valid. So the above example still holds, but the data are not misaligned.

@michael-platzer
Copy link
Contributor

Unfortunately, the address alignment constraints for vector loads and stores are currently very restrictive. Although 8-bit data is never misaligned, any vector load or store with a base address that is not aligned to the width of the memory interface will result in a misaligned memory request. Currently, the memory requests of Vicuna are directly handled by the memory bus, which is incapable of handling misaligned requests.

The current draft for the CORE-V-XIF standardized co-processor interface for RISC-V cores intends to route memory requests of a co-processor through the main core and to handle misaligned memory requests in the main core. However, that functionality is not implemented yet (and the specification might still change).

The current workaround is to ensure that the base address of any vector load and store is aligned to the width of the memory interface (e.g.., if the memory interface is 64 bits wide, the base address should be a multiple of 8).

@stevobailey
Copy link
Contributor Author

Hey, any updates on this? I saw the merge happened. Thanks for your work on it!
openhwgroup/cv32e40x#440

@michael-platzer
Copy link
Contributor

Hi @stevobailey! Yes, with the latest commit I just lifted the requirement that the base address of vector loads and stores needs to be aligned to the width of the memory interface when using the XIF memory interface. Currently, this interface can only be used when CV32E40X is the main core and the memory interface width (VMEM_W) is configured to 32 bits (although in the future CV32E40X should gain the capability to support an XIF memory interface with arbitrary data width).

Unfortunately, the vector CSR cannot be accessed when using CV32E40X as the main core yet. I will focus on this next.

@stevobailey
Copy link
Contributor Author

Thanks! Are you planning on porting this fix to Ibex as well?

@michael-platzer
Copy link
Contributor

That would require implementing the XIF interface for Ibex (including the XIF memory interface). There has been some work to add support for this to Ibex here: https://github.com/lowRISC/ibex/commits/cvxif . However, it appears that there are still issues with this implementation, as most CI checks are failing. Also, it was based on an earlier version of the XIF spec that did not include the memory interface.

Unfortunately, the fix cannot be ported unless the Ibex core gains an up-to-date XIF interface. While I would love to see support for the XIF interface added to Ibex, I do not have resources to work on this right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants