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

Question about registerfile #46

Closed
moimfeld opened this issue Mar 17, 2022 · 2 comments
Closed

Question about registerfile #46

moimfeld opened this issue Mar 17, 2022 · 2 comments
Labels
question Further information is requested

Comments

@moimfeld
Copy link
Contributor

Hi @michael-platzer

This is Moritz, I will be doing the verification of Vicuna. I am currently setting up a simulation environment for Questasim to replicate your tests, and now I have a question about the register file.

Since I am targeting ASIC I also want to add an option for an ASIC register file. I have a working version of the ASIC registerfile here: https://github.com/moimfeld/vicuna/blob/asic_dev/rtl/vproc_vregfile.sv .

But my question is if the XORs in the following lines are only there for the correct functionality of the "XOR" RAM, or if they have some other purpose (like for example masking)?

wr_data = wr_data ^ rd_data[PORTS_RD + gw - ((i < gw) ? 1 : 0)][i + ((i < gw) ? 0 : 1)];

rd_data_o[i] = rd_data_o[i] ^ rd_data[i][j];

I ask because in my ASIC version I don't use this XORs at the moment and I want to make sure that I don't lose some functions.

@michael-platzer
Copy link
Contributor

Hi @moimfeld , thank you very much for the work you are doing!

The XORs are only required for the correct functionality of the XOR RAM as described here: https://tomverbeure.github.io/2019/08/03/Multiport-Memories.html#xor-based-multi-port-rams

If you are implementing a RAM that can support multiple write ports without requiring that the entire register file is replicated for each write port, then these are not required. An important feature that the register file must support is that register data can be written at byte granularity, based on the byte enable signal, but I see that you have implemented that in your ASIC version.

Please let me know if you have any other questions or if there is anything that you need or things that you would like to discuss!

@michael-platzer michael-platzer added the question Further information is requested label Mar 17, 2022
@moimfeld
Copy link
Contributor Author

Hi @michael-platzer

Thanks a lot for your answer, this resolves my question.
I will open a pull-request once the Questasim environment has passed all tests. There will be some small RTL changes that need to be discussed once I open the PR (just for your information). But for now everything is clear as your code is very well structured and easy to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants