search

cms-L1TK / project_generation_scripts

Python scripts to generate the wiring map of the tracklet pattern recognition & the top-level HDL that calls the HLS modules in the Hybrid Chain.
1 stars 2 forks source link

IR-VMR Chain #23

Closed meisonlikesicecream closed 3 years ago

meisonlikesicecream commented 3 years ago

Added implementation for the IR and the VMR. The scripts can now handle two-dimensional arrays of memories, provided that they are partitioned. It can also handle mixed output memories, i.e. memories of a certain type where some get read further down the chain while others don't. Intermediate final memories remain in the top-level, albeit unconnected to the interface, thus will be deleted by Vivado. When running with the -x flag, all intermediate memories gets ports on the interface (worth noting that the resource usage will differ compared to the default script as no memories will be deleted by vivado in this case). The inputs ports (DTCLinks) to the IRs are connected directly to the interface.

The simplest chain "-uut VMR_L2PHIA -u 1 -d 0", which contains two IRs and one VMR has been successfully synthesised and implemented in Vivado. No test bench has been created yet.

NOTE: the scripts use the wires.dat, processingmodules.dat, and memorymodules.dat from https://github.com/cms-L1TK/firmware-hls/pull/151 (downloaded by download.sh, then found in /LUTs); and the IR from https://github.com/cms-L1TK/firmware-hls/tree/IR_moveLUTs, thus should not be merged until those HLS firmware branches has been merged.

NOTE 2: To implement and synthesise the IR-VMR chain that was created by the scripts, some minor edits has to be done to the firmware HLS. A PR for those changes will be made.

tomalin commented 3 years ago

I've verified that the VHDL of the PR-ME-MC chain is unaffected by this new branch, when used with the "master" of HLS. However when used with the new wires.dat file, it does chain. e.g. PR_L3PHIC now reads 10 TPROJ memories instead of 8. This will be a problem, as HLS PR assumes only 8 TPROJ memories. I've raised this issue in https://github.com/cms-L1TK/firmware-hls/pull/151#issuecomment-850362307 , since it's not caused by your PR.

tomalin commented 3 years ago

Please update text describing this PR. I believe it doesn't need "wires.py". Instead, after checking out the HLS code from IR_moveLUTS, one should copy into emData/ the version of download.sh from HLS PR 151; then run there the command "download.sh -t" to download the files wires.dat, processingmodules.dat & memorymodules.dat, which should then be copied to project_generation_scripts/.

tomalin commented 3 years ago

Add "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0" as another example command to README.md.

tomalin commented 3 years ago

TrackletGraph.pdf the outputs of VMR_L2PHIA overlap. Can TrackletGraph.py be modified to prevent this, whilst not messing up the .pdf of PR-ME-MC?

tomalin commented 3 years ago

If easy, add to memUtil_pkg.vhd for each memory something like, "constant VMSTE_22_width : natural := 22; constant VMSTE_22_pages : natural := 2;". And for binned memories add a constant for the number of bins. This will avoid need to hard-wire these constants in VHDL test-bench.

tomalin commented 3 years ago

memUtil_pkg.vhd defines both t_arr_DL_39_empty_neg and t_arr_DL_39_read to be exactly the same type "array(enum_DL_39) of std_logic". We therefore don't need two types. I suggest a single type named "t_arr_DL_39_1b".

meisonlikesicecream commented 3 years ago

TrackletGraph.pdf the outputs of VMR_L2PHIA overlap. Can TrackletGraph.py be modified to prevent this, whilst not messing up the .pdf of PR-ME-MC?

I've tried adding a fix for this now (based on whatever was already in TrackletGraph.py), although the text for the VMR becomes quite small... https://github.com/cms-L1TK/project_generation_scripts/pull/23/files#diff-67bcb68c62e646510a9286c8eed7bf88f48295cb2252257e762e4e38a6d6c3c5R854

meisonlikesicecream commented 3 years ago

@tomalin Do you want to take another look now when I have rebased with master including the TE-TC chain? Thanks to Rui's changes, some of the ugliness could be removed, and the last bug where the TE Outer memories were incorrectly assumed to be unbinned was easily fixed. (the reasons why I wanted to wait for that PR :-) )

tomalin commented 3 years ago

If I do "./generator_hdl.py --uut TC_L1L2E -u 1 -d 0 -x" , with wires.dat etc. produced by Wires.py, then the resulting SectorProcessorFull.vhd differs if I use ir_vmr_chain instead of master. Specifically, the order of the signals assigned to the innerStubs ports of work.TC_L1L2E differs. Is this intentional?

tomalin commented 3 years ago

If I run "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0", I get warning messages that InputRouter & VMRouter "template parameters are not implemented yet"

tomalin commented 3 years ago

The txt in TrackletProject.pdf is still too small for the IR-VMR chain. Is it possible to make it larger, without messing up the visibility of this file for PR-ME-MC & TE-TC?

meisonlikesicecream commented 3 years ago

If I run "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0", I get warning messages that InputRouter & VMRouter "template parameters are not implemented yet"

We never used the template parameters for anything, and I can't see any potential use of them. The VMR has got a lot of template parameters, so I saw no point of wasting time on adding that. I could just remove the printed warning and only return an empty string if you would prefer that, unless you give me a good reason to why we need the template parameters in the first place...

tomalin commented 3 years ago

If I run "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0", I get warning messages that InputRouter & VMRouter "template parameters are not implemented yet"

We never used the template parameters for anything, and I can't see any potential use of them. The VMR has got a lot of template parameters, so I saw no point of wasting time on adding that. I could just remove the printed warning and only return an empty string if you would prefer that, unless you give me a good reason to why we need the template parameters in the first place...

Your proposed solution sounds fine. I you believe template parameters are not needed, then we don't need a warning message about them.

meisonlikesicecream commented 3 years ago

If I do "./generator_hdl.py --uut TC_L1L2E -u 1 -d 0 -x" , with wires.dat etc. produced by Wires.py, then the resulting SectorProcessorFull.vhd differs if I use ir_vmr_chain instead of master. Specifically, the order of the signals assigned to the innerStubs ports of work.TC_L1L2E differs. Is this intentional?

Ah, changing the the order for the TC was not intentional. I've changed it so the sorting only affects the VMSME and VMSTE memories now as the VMR needs the VMSTE memories in the correct order.

I just double checked to see if we actually needed the sorting, and it seems like the wires.dat from the emulation already is in the correct order straight from the file now. But I swear this hasn't always been the case (unfortunately I don't have any proof), so I think I'll keep the sorting part as safety in case that would change in the future....

meisonlikesicecream commented 3 years ago

The txt in TrackletProject.pdf is still too small for the IR-VMR chain. Is it possible to make it larger, without messing up the visibility of this file for PR-ME-MC & TE-TC?

After some trial and error it should be increased now, while the TETC and PRMEMC remain mostly unchanged.

tomalin commented 3 years ago

If I do "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0 -x", then in SectorProcessorFull.vhd , the signal VMSME_16_mem_AV_din is written to memory VMSME_16, but it's not produced by anything. The VMR only seems to have output ports connected to the AS memories.

meisonlikesicecream commented 3 years ago

If I do "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0 -x", then in SectorProcessorFull.vhd , the signal VMSME_16_mem_AV_din is written to memory VMSME_16, but it's not produced by anything. The VMR only seems to have output ports connected to the AS memories.

Ah, I found the issue, assuming you're using the default wires.dat. It's because the portnames in for the ME and TE memories aren't the same in the default wiring created by these scripts and the ones created by the emulation (which I'm using for the IR-VMR chain). So the portmatching fails for those memories, thus they won't be written out, and you might also notice that it doesn't include the IR in this case. Because this whole PR is assuming you're using the emulation wiring, I'm not sure if I should do anything about that...

tomalin commented 3 years ago

If I do "generator_hdl.py --uut VMR_L2PHIA -u 1 -d 0 -x", then in SectorProcessorFull.vhd , the signal VMSME_16_mem_AV_din is written to memory VMSME_16, but it's not produced by anything. The VMR only seems to have output ports connected to the AS memories.

Ah, I found the issue, assuming you're using the default wires.dat. It's because the portnames in for the ME and TE memories aren't the same in the default wiring created by these scripts and the ones created by the emulation (which I'm using for the IR-VMR chain). So the portmatching fails for those memories, thus they won't be written out, and you might also notice that it doesn't include the IR in this case. Because this whole PR is assuming you're using the emulation wiring, I'm not sure if I should do anything about that...

Confirmed. I was taking download.sh from a old version of https://github.com/cms-L1TK/firmware-hls/pull/151 . With latest one, problem goes away.