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

Updates to remove the generate statements #54

Closed aryd closed 7 months ago

aryd commented 10 months ago

This PR modifies the code such that the memory modules are not instantiated using "generate" statements, but rather each instance is written out in the SectorProcessor.vhd file. This was already done for the processing modules so this was not as large a change as I had originally thought. This change will then allow to simplify how we implement the memory modules where we will be able to have different number of memory copies for the combined memory modules.

aryd commented 8 months ago

Andrew,

I think this must be memMod.inst. I would need to logon and check, but I think this is what I did in other instances. I guess this code was not executed? Otherwise I should have seen an error?

-Anders

Anders Ryd @.**@.>

On Nov 7, 2023, at 12:19 PM, Andrew Hart @.**@.>> wrote:

@aehart commented on this pull request.

In WriteVHDLSyntax.pyhttps://github.com/cms-L1TK/project_generation_scripts/pull/54#discussion_r1384765079:

  • string_mem += " NUM_PAGES".ljust(str_len)+"=> " + str(2**bxbitwidth) + "\n"
  • string_mem += " )\n"
  • string_mem += " port map (\n"
  • string_mem += " CLK".ljust(str_len)+"=> CLK,\n"
  • string_mem += " ADDR".ljust(str_len)+"=> "+mtypeB+"_mem_AV_readaddr(var),\n"
  • string_mem += " DATA".ljust(str_len)+"=> "+mtypeB+"_mem_AV_dout(var),\n"
  • string_mem += " READ_EN".ljust(str_len)+"=> "+mtypeB+"_mem_A_enb(var),\n"
  • string_mem += " NENT_ARR".ljust(str_len)+"=> "+mtypeB+"_mem_AA" + ("A" if is_binned else "") + "V_dout_nent(var),\n"
  • string_mem += " DONE".ljust(str_len)+"=> "+proc+"_DONE\n"
  • string_mem += " );\n"
  • string_mem += " end generate "+mtypeB+"_loop;\n\n"
  • memList = memDict[mtypeB]
  • for memMod in memList:
  • mem = memMod.list

@arydhttps://github.com/aryd The MemModule class doesn't have a member called list. Did you mean inst?

⬇️ Suggested change

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/project_generation_scripts/pull/54#pullrequestreview-1717446529, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTI7BRICP4LL5H35C5TYDIKNNAVCNFSM6AAAAAA5N3VFKOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJXGQ2DMNJSHE. You are receiving this because you were mentioned.Message ID: @.***>

aehart commented 8 months ago

I think this must be memMod.inst. I would need to logon and check, but I think this is what I did in other instances. I guess this code was not executed? Otherwise I should have seen an error?

@aryd I only noticed this because I updated the submodule in cms-L1TK/firmware-hls#298 so that we could see if the CI tests were successful. They failed as soon as emData/download.sh was executed…

So this code is definitely executed, at least by the calls to generator_hdl.py in that script.

aryd commented 8 months ago

I’ll have to check - I thought I had tested this…I will take a look later today.

-Anders

Anders Ryd @.**@.>

On Nov 7, 2023, at 1:53 PM, Andrew Hart @.**@.>> wrote:

I think this must be memMod.inst. I would need to logon and check, but I think this is what I did in other instances. I guess this code was not executed? Otherwise I should have seen an error?

@arydhttps://github.com/aryd I only noticed this because I updated the submodule in cms-L1TK/firmware-hls#298https://github.com/cms-L1TK/firmware-hls/pull/298 so that we could see if the CI tests were successful. They failed as soon as emData/download.sh was executed…

So this code is definitely executed, at least by the calls to generator_hdl.py in that script.

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/project_generation_scripts/pull/54#issuecomment-1798444842, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTN56EH3S6CETEMGSEDYDIVNDAVCNFSM6AAAAAA5N3VFKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJYGQ2DIOBUGI. You are receiving this because you were mentioned.Message ID: @.***>

aryd commented 8 months ago

Andrew,

I looked into this now. I concluded that .list should be changed to .inst. Now the download.sh script runs.

I pushed the fix to the branch RemoveGenerate in project_generation_script. As the PR has already been closed I presume I should open a new PR?

I also pushed the new project_generatio_script version to the branch RemoveGenerate in the main HLS repo.

(I’m not 100% sure why I missed this this, but I find that working with the submodules and the download.sh script is very confusing as every time you run the download.sh script it reinitializes the submodule and changes the branch you have checked out in the project_generation_script. I spent 1/2 hour this morning being confused about this as the code in my project_generation_scripts are changed every time I read download.sh….)

-Anders

Anders Ryd @.**@.>

On Nov 7, 2023, at 1:53 PM, Andrew Hart @.**@.>> wrote:

I think this must be memMod.inst. I would need to logon and check, but I think this is what I did in other instances. I guess this code was not executed? Otherwise I should have seen an error?

@arydhttps://github.com/aryd I only noticed this because I updated the submodule in cms-L1TK/firmware-hls#298https://github.com/cms-L1TK/firmware-hls/pull/298 so that we could see if the CI tests were successful. They failed as soon as emData/download.sh was executed…

So this code is definitely executed, at least by the calls to generator_hdl.py in that script.

— Reply to this email directly, view it on GitHubhttps://github.com/cms-L1TK/project_generation_scripts/pull/54#issuecomment-1798444842, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABI4LTN56EH3S6CETEMGSEDYDIVNDAVCNFSM6AAAAAA5N3VFKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJYGQ2DIOBUGI. You are receiving this because you were mentioned.Message ID: @.***>

aehart commented 8 months ago

I pushed the fix to the branch RemoveGenerate in project_generation_script. As the PR has already been closed I presume I should open a new PR?

This PR is still open, so I will take a look at it and approve if everything is OK now.

I also pushed the new project_generatio_script version to the branch RemoveGenerate in the main HLS repo.

Thanks. The CI jobs failed with some errors that look transient. I just restarted them, so that we can see if they're happy with the changes to SectorProcessor.