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

refac: (in hpdcache interface) separate the modules for dcache and icache #2173

Merged
merged 9 commits into from
Jun 11, 2024

Conversation

takeshiho0531
Copy link
Contributor

@takeshiho0531 takeshiho0531 commented May 31, 2024

for GSoC Project: Transforming the OpenHW High Performance Data Cache into a High Performance Instruction Cache

  • what:

    • I moved the parts of the D$ instantiation in cva6_hpdcache_subsystem.sv (the parts involving cva6_hpdcache_cmo_if_adapter, hwpf_stride_wrapper, and hpdcache, excluding the I$ instantiation and AXI arbiter instantiation) into a single module called cva6_hpdcache_wrapper and placed it in a separate file (cva6_hpdcache_wrapper.sv).
    • In the cva6_hpdcache_wrapper module, new ports were added to connect with the input and output ports of the AXI arbiter instantiation.
  • purpose:

    • To clarify the area of the dcache
    • To enable the use of hpdcache as icache, (I think) it is better to modularize the part where hpdcache is currently used as dcache, so that this module can also be called by icache in the future.
    • PR feat: extended hpdc cache subsystem #1978 (I aborted this PR) is too large to debug easily, so it has been split.
  • TODO

  • What's next?

    • extend cva6_hpdcache_if_adapter so that it can also convert input request/response to the appropriate type for hpdcache when using hpdcache as icache. (ex. input type:icache_req_t → output type: hpdcache_req_t)

core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
core/cache_subsystem/dcache.sv Outdated Show resolved Hide resolved
Copy link
Contributor

❌ failed run, report available here.

1 similar comment
Copy link
Contributor

❌ failed run, report available here.

@takeshiho0531 takeshiho0531 changed the title Draft: for adapting extended hpdc Draft: for applying extended hpdc May 31, 2024
@takeshiho0531 takeshiho0531 changed the title Draft: for applying extended hpdc feat: (in hpdcache) separate the modules for dcache and icache May 31, 2024
@takeshiho0531 takeshiho0531 changed the title feat: (in hpdcache) separate the modules for dcache and icache refac: (in hpdcache) separate the modules for dcache and icache May 31, 2024
@takeshiho0531 takeshiho0531 changed the title refac: (in hpdcache) separate the modules for dcache and icache refac: (in hpdcache interface) separate the modules for dcache and icache Jun 1, 2024
@takeshiho0531 takeshiho0531 changed the title refac: (in hpdcache interface) separate the modules for dcache and icache draft: (in hpdcache interface) separate the modules for dcache and icache Jun 2, 2024
@takeshiho0531 takeshiho0531 reopened this Jun 4, 2024
@takeshiho0531 takeshiho0531 changed the title draft: (in hpdcache interface) separate the modules for dcache and icache refac: (in hpdcache interface) separate the modules for dcache and icache Jun 4, 2024
Copy link
Contributor

github-actions bot commented Jun 4, 2024

❌ failed run, report available here.

@takeshiho0531 takeshiho0531 marked this pull request as ready for review June 5, 2024 01:14
@takeshiho0531 takeshiho0531 marked this pull request as draft June 5, 2024 01:16
Copy link
Contributor

github-actions bot commented Jun 5, 2024

❌ failed run, report available here.

@takeshiho0531 takeshiho0531 marked this pull request as ready for review June 7, 2024 09:47
@takeshiho0531
Copy link
Contributor Author

@cfuguet @AileonN

I have some questions...

  • I think I need to add some assertions for some input types of module cva6_hpdcache_wrapper(ex. hpdcache_mem_req_t) since they need to fulfill certain macros. Is it better to add some macros for assertions corresponding to the macros(ex. HPDCACHE_TYPEDEF_MEM_ATTR_T) in hpdcache_typedef.svh?
  • I'm not sure how to solve these errors(↓) in (git-lab?) ci.... They occurred when testing ASIC Synthesis cv32a6_embedded.
Error: ../..//core/cache_subsystem/cva6_hpdcache_subsystem.sv:179: Unable to open file `hpdcache_typedef.svh': No such file or directory. (VER-41)
Error: ../..//core/cache_subsystem/cva6_hpdcache_subsystem_axi_arbiter.sv:173: Package 'hpdcache_pkg' has not been analyzed for import or content extraction. (VER-224)
Error: ../..//core/cache_subsystem/cva6_hpdcache_subsystem_axi_arbiter.sv:173: The symbol 'HPDCACHE_MEM_READ' is not defined. (VER-956)
Error: ../..//core/cache_subsystem/cva6_hpdcache_subsystem_axi_arbiter.sv:174: Package 'hpdcache_pkg' has not been analyzed for import or content extraction. (VER-224)
Error: ../..//core/cache_subsystem/cva6_hpdcache_subsystem_axi_arbiter.sv:174: invalid symbol hpdcache_mem_atomic_e found in expression. (VER-255)
Error: ../..//core/cache_subsystem/cva6_hpdcache_if_adapter.sv:17: Package 'hpdcache_pkg' has not been analyzed for import or content extraction. (VER-224)
Error: ../..//core/cache_subsystem/cva6_hpdcache_if_adapter.sv:17: Syntax error at or near token 'hpdcache_cfg_t'. (VER-294)
Error: ../..//core/cache_subsystem/cva6_hpdcache_wrapper.sv:15: Unable to open file `hpdcache_typedef.svh': No such file or directory. (VER-41)

@cfuguet
Copy link
Contributor

cfuguet commented Jun 10, 2024

I'm not sure what is the issue here... it looks like the +incdir directive did not work for the ASIC synthesis. The tool is not finding the hpdcache_typedef.svh header file in the HPDcache subdirectory. All the following errors are the consequence of this.

@JeanRochCoulon could you please share the synthesis report ?

@takeshiho0531 is working in the GSoC project to adapt the HPDcache and enable its usage as an Instruction Cache as well. As a first step, she proposes this refactoring of the hpdcache subsystem which simplifies greatly the future integration of the HPDcache as Icache of the CVA6

@JeanRochCoulon
Copy link
Contributor

Maybe I see the problem.
The Asic synthesis flow has been updated last week.
can you rebase your PR to update the flow ?

@cfuguet
Copy link
Contributor

cfuguet commented Jun 10, 2024

Maybe I see the problem.

The Asic synthesis flow has been updated last week.

can you rebase your PR to update the flow ?

@takeshiho0531 can you rebase ?

Copy link
Contributor

✔️ successful run, report available here.

@cfuguet
Copy link
Contributor

cfuguet commented Jun 11, 2024

Hi @takeshiho0531,

I think your PR is ready to be merged, but the commits messages are not convenient. Could you please rebase your PR and make a single commit with an appropriate message ?

Thanks !

@JeanRochCoulon
Copy link
Contributor

Hello @takeshiho0531
Thank for your contributions.
I am ready to merge. Tell me !

@cfuguet
Copy link
Contributor

cfuguet commented Jun 11, 2024

Hi @JeanRochCoulon, I think you can merge @takeshiho0531 contribution. You are doing a merge right ? What is the message of the merge commit ? It is just that there are 9 commits with no so useful descriptions, but if the message is the one in the PR description that is ok.

@cfuguet
Copy link
Contributor

cfuguet commented Jun 11, 2024

However, I would change the description of the PR to :

refactor hpdcache_cache_subsystem module code to ease reutilization

@JeanRochCoulon JeanRochCoulon merged commit bc7149a into openhwgroup:master Jun 11, 2024
10 checks passed
@JeanRochCoulon
Copy link
Contributor

The commits are squashed, and I set your description.

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

Successfully merging this pull request may close these issues.

None yet

3 participants