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

[P2] Installation and Data Preloading Issue #11

Open
shourovrm opened this issue Aug 31, 2023 · 7 comments · Fixed by #36
Open

[P2] Installation and Data Preloading Issue #11

shourovrm opened this issue Aug 31, 2023 · 7 comments · Fixed by #36
Assignees
Labels
new feature New feature or request test

Comments

@shourovrm
Copy link

Hi Tan!

1) Installation instructions considering the current changes

I have been checking your CGRA-Flow repository (alongside its sub-modules like VectorCGRA, CGRA-Mapper etc.) for the past few months. Its great to see that you have integrated vcd generation recently. However, I've noticed that the current installation instructions for both VectorCGRA are outdated due to these new changes.

For example, when testing the TileRTL_test.py, it seems that the mentioned version of pymtl3 in the README does not include the VerilogTranslationPass package (it rather has the TranslationImportPass package).

So, could you please update the installation instructions by specifying the correct pymtl3 version that you are using? It would also be great if the mentioned docker container is also updated to integrate the changes.

2) Preloading data to CGRA

I previously formulated a simpler design flow, which uses the config.json generated by CGRA-Mapper, pass its src_opt following the CGRATemplateRTL_test, and later generate VCD files from it.

However, I'm struggling to figure out how to preload data into the tiles and the SPM. Specifically, I tried preloading constants and input values for the FIR test but was unable to see these values in the generated VCD file. On the other hand, simpler tests like VectorAdderRTL_test show both input and output data clearly in the VCD.

Would you please provide some guidance or documentation on how to preload data into the CGRA tiles and SPM for and test its output?

@tancheng
Copy link
Owner

tancheng commented Sep 1, 2023

Hi shourovrm,

  1. We will use the updated PyMTL (i.e., pip install -U git+https://github.com/cornell-brg/pymtl3@yo-struct-list-fix) for modeling and translation. The TileRTL_test.py should be able to generate Verilog, please make sure the --test-verilog is attached after the TileRTL_test.py, for example, pytest --tb=short -sv ../TileRTL_test.py --test-verilog. If it still doesn't work, please let me know. I will update the README next month to make it more clear.
    For the docker, I am not sure whether it requires this latest change. The docker is just to show the demo with the previous version and it should work with the old version. But I will try to update all the related stuff if i have time.

  2. For data preloading into SPM, it should okay if the preload data can be sent through the pytest. However, I didn't test it with the RTL version, the provided FIR test is in CL rather than RTL. Do you try the FIR in RTL simulation? The input/output can be seen in AdderRTL as the data is sent into the port and sent out via the outport. However, in the CGRA modeling, the inputs for now is like constants/wires implicitly writing into the data memory/registers. It is not like a data sent into a port then update the SPM in each cycle. I guess that's why the VCD doesn't include it. In short, the current design doesn't model the data preloading in a good way. We can think about how to make an appropriate design for the data feeding. For your kernel test/verification, you need to check whether the final data/value stored in the specific address/location in the SPM is expected or not, which is similar to the FIR CL test, but I am not sure how to do it by looking into VCD.

Thanks for your questions again. Will update the README.

@tancheng tancheng linked a pull request Dec 6, 2024 that will close this issue
1 task
@tancheng tancheng self-assigned this Dec 24, 2024
@tancheng tancheng added the new feature New feature or request label Dec 24, 2024
@tancheng
Copy link
Owner

tancheng commented Dec 24, 2024

TODO:

  • Enable preloading ctrl signals via controller.
  • Enable preloading data into data memory via controller. I don't think this is necessary, SRAM can always be hacked.
  • Enable preloading const data into tile register via controller.

@tancheng tancheng assigned yyan7223 and unassigned tancheng Dec 24, 2024
@tancheng tancheng changed the title Installation and Data Preloading Issue [P2] Installation and Data Preloading Issue Jan 3, 2025
@tancheng tancheng added the test label Jan 3, 2025
@tancheng tancheng assigned tancheng and unassigned yyan7223 Jan 4, 2025
@tancheng
Copy link
Owner

tancheng commented Jan 4, 2025

Hi @yuqisun, I think this would be much easier than #22, and would help you understand and get familiar with how the constant/parameter is pre-stored in the ConstQueue. Would you be interested in finishing this issue/feature first? If not, I can take this over.

Basically, we can rename recv_ctrl_pkt_queue and send_to_ctrl_ring_ctrl_pkt

s.recv_ctrl_pkt_queue.send //= s.send_to_ctrl_ring_ctrl_pkt
to recv_pkt_queue and send_to_cgra_ring_pkt, respectively, and make them not only be able to send ctrl pkt, but also be able to send const pkt. Once each tile receives the pkg, const would be pushed into the ConstQueue
s.const_queue = [Wire(DataType) for _ in range(num_const)]
It shouldn't be a bunch of wires, instead, it should be a register file like the ctrl memory:
s.reg_file = RegisterFile(CtrlSignalType, ctrl_mem_size, 1, 1)

How does this sound?

@yuqisun
Copy link
Collaborator

yuqisun commented Jan 4, 2025

Hi, thanks and please let me try first.

Do I need wait for your clean and en/rdy PR then fork from master?
And how can I verify this, runs ControllerRTL_test.py good?

@yuqisun
Copy link
Collaborator

yuqisun commented Jan 4, 2025

Hi Cheng,

I draw a flow graph for the ControllerRTL, could you help double review if anything wrong? And what I need to do is the add extra type (ConstantPktType which can use DataType directly?) from cpu to controller right?

Then save both CtrlPktType and DataType in RegisterFile? And connect reg_file.rdata[0] with something, and update reg_file in @update?

Screenshot from 2025-01-04 20-31-18

@tancheng
Copy link
Owner

tancheng commented Jan 4, 2025

And what I need to do is the add extra type (ConstantPktType which can use DataType directly?) from cpu to controller right?

Correct!

Then save both CtrlPktType and DataType in RegisterFile?

Nope. The DataType needs to go to https://github.com/tancheng/VectorCGRA/blob/master/mem/const/ConstQueueRTL.py. So we need to add register_file into ConstQueue first (mimic CtrlMemDynamicRTL.py).

Besides the data memory you draw, the controller also connects to another ctrl_ring (instead of the router u draw that is belonging to the multi-cgra router), and the ctrl_ring connects its each router to a ctrl memory via tile.

@tancheng tancheng moved this to Todo in Scaling up/out CGRA Jan 10, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
yuqisun added a commit to yuqisun/VectorCGRA that referenced this issue Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request test
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants