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

Altera opt 1 #2592

Merged
merged 24 commits into from
Nov 15, 2024
Merged

Altera opt 1 #2592

merged 24 commits into from
Nov 15, 2024

Conversation

AngelaGonzalezMarino
Copy link
Contributor

@AngelaGonzalezMarino AngelaGonzalezMarino commented Nov 8, 2024

The first optimization for Altera FPGA is to move the instruction queue to LUTRAM. The reason why the optimization previously done for Xilinx is not working, is that in that case asynchronous RAM primitives are used, and Altera does not support asynchronous RAM. Therefore, this optimization consists in using synchronous RAM for the instruction queue and FIFOs inside wt axi adapter.

The main changes to the existing code are:

  • New RAM module to infer synchronous RAM in altera with independent read and write ports (SyncDpRam_ind_r_w.sv)

  • Changes inside cva6_fifo_v3 to adapt to the use of synchronous RAM instead of asynchronous:

  • When the FIFO is not empty, next data is always read and available at the output hiding the reading latency introduced by synchronous RAM (similar to fall-through approach). This is a simplification that is possible because in a FIFO we always know what is the next address to be read.

  • When data is read right after write, we can’t use the previous method because there is a latency to first write the data in the FIFO, and then to read it. For this reason, in the new design there is an auxiliary register used to hide this latency. This is used only if the FIFO is empty, so we detect when the word written is first word, and keep it in this register. If the next cycle comes a read, the data out is taken from the aux register. Afterwards the data is already available in the RAM and can be read continuously as in the first case.

All this is only used inf FpgaAlteraEn parameter is enabled, otherwise the previous implementation with asynchronous RAM applies (when FpgaEn is set), or the register based implementation (when FpgaEn is not set).

AngelaGonzalezMarino and others added 3 commits November 8, 2024 17:52
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Contributor

github-actions bot commented Nov 8, 2024

❌ failed run, report available here.

Copy link
Contributor

✔️ successful run, report available here.

Copy link
Contributor

❌ failed run, report available here.

Copy link
Contributor

❌ failed run, report available here.

1 similar comment
Copy link
Contributor

❌ failed run, report available here.

Copy link
Contributor

❌ failed run, report available here.

Copy link
Contributor

@JeanRochCoulon JeanRochCoulon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
But @Gchauvon can you have a look to this PR ?

Copy link
Contributor

❌ failed run, report available here.

1 similar comment
Copy link
Contributor

❌ failed run, report available here.

@Gchauvon
Copy link
Contributor

LGTM

@JeanRochCoulon
Copy link
Contributor

Hello @AngelaGonzalezMarino The ASIC synthesis produces 1.2 more kgates. Do you know why ?

@JeanRochCoulon
Copy link
Contributor

I get an hypothesis: some registers have been added in cva6_fifo_v3. They need to be gated by the FPGA Altera Enable parameter.

@AngelaGonzalezMarino
Copy link
Contributor Author

I get an hypothesis: some registers have been added in cva6_fifo_v3. They need to be gated by the FPGA Altera Enable parameter.

maybe, let me try

Copy link
Contributor

❌ failed run, report available here.

Co-authored-by: JeanRochCoulon <[email protected]>
Copy link
Contributor

❌ failed run, report available here.

@JeanRochCoulon JeanRochCoulon merged commit 33c5d77 into openhwgroup:master Nov 15, 2024
10 checks passed
@AngelaGonzalezMarino
Copy link
Contributor Author

thanks!!

Gchauvon added a commit to ThalesSiliconSecurity/cva6 that referenced this pull request Jan 22, 2025
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.

3 participants