Skip to content

ZSTD decoder: cleanups, new sequence executor and fixes for subcomponents#3636

Open
magancarz wants to merge 94 commits intogoogle:mainfrom
antmicro:zstd_decoder_pr
Open

ZSTD decoder: cleanups, new sequence executor and fixes for subcomponents#3636
magancarz wants to merge 94 commits intogoogle:mainfrom
antmicro:zstd_decoder_pr

Conversation

@magancarz
Copy link
Copy Markdown
Contributor

Added functionality:

  • Cleaned up the ZSTD decoder and cocotb tests environment
  • Fixed the ZSTD decoder subcomponents
  • Added a detailed testing environment with intermediate values checks
  • Added a CLI tool for running decoder test cases
  • Refactored the sequence executor architecture
  • Added fixes for parameterizing AXI buses width
  • Added a verilog model for 2RW ram
  • Added Verilator as a new dependency to improve cocotb tests runtime
  • Updated README
    • Updated and added new information about the components of the ZSTD Decoder
    • Added diagrams to present the updated structure
  • Added a workaround for a bug causing the xls_benchmark_ir targets to fail

Supersedes #3035

@google-cla
Copy link
Copy Markdown

google-cla Bot commented Dec 31, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@proppy
Copy link
Copy Markdown
Member

proppy commented Mar 3, 2026

@rw1nkler @magancarz sorry I let that slip, can you rebase?

magancarz and others added 21 commits March 16, 2026 15:05
…d channels; update flags, channel names and top names."

This reverts commit 77dcc81.
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Add cocotb testing utilities:
- XLSStruct for easier handling and serializing/deserializing XLS structs
- XLSChannel that serves as a dummy receiving channel
- XLSMonitor that monitors transactions on an XLS channel
- XLSDriver that can send data on an XLS channel
- LatencyScoreboard that can measure latency between corresponding transactions on input and output buses
- File-backed AXI memory python model

Add cocotb tests for:
- modules/zstd/memory/MemReader
- modules/zstd/memory/AxiWriter
- modules/zstd/memory/MemWriter

Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Robert Winkler <rwinkler@antmicro.com>
Co-authored-by: Michal Czyz <mczyz@antmicro.com>
Signed-off-by: Michal Czyz <mczyz@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Krzysztof Obłonczek <koblonczek@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
This reverts commit 04ad379225b706ddf492d440c673e77348d7a409.
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Signed-off-by: Krzysztof Obłonczek <koblonczek@antmicro.com>
ZstdDecoder:
  * Cocotb tests:
    * Move third-party verilog modules (AXI Interconnect) to external directory
    * Replace AXI Interconnect with AXI Crossbar that handles simultaneous AXI Read
      and Write transactions
    * Add reference memory and fill it with expected data for comparison
      against testbench memory at the end of the decoding

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Adjust cocotb test to:
* Removal of Repacketizer proc
* Removal of stream-based output channels from
  * SequenceExecutor
  * ZstdDecoder

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
* Decode multiple ZSTD frames in a single cocotb testbench
* Add one cocotb testbench per type of the ZSTD frames:
  * Frames with RAW blocks only
  * Frames with RLE blocks only
  * Frames with Compressed blocks only (disabled)
  * Frames with mixed blocks (disabled)

Internal-tag: [#67272]
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Caused by timeouts after rebase - looks like regression in the
performance of codegen

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
…sources

Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Improve formatting, wording and fix lint issues

Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Signed-off-by: Wojciech Sipak <wsipak@antmicro.com>
Co-authored-by: Pawel Czarnecki <pczarnecki@antmicro.com>
Co-authored-by: Krzysztof Oblonczek <koblonczek@antmicro.com>
Co-authored-by: Wojciech Sipak <wsipak@antmicro.com>
Co-autored-by: Dominik Lau <dlau@antmicro.com>
Co-authored-by: Szymon Gizler <sgizler@antmicro.com>

Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
sgizler and others added 24 commits March 17, 2026 08:40
…calculation

As defined in https://datatracker.ietf.org/doc/html/rfc8878#section-4.2.1.1
raw weights consume ceil(n_symb/2) bytes.

The correct equivalent of that ceiled div is:
(n_symb+1)>>1
rather than
(n_symb>>1)+1
we previously used
…t weight

Previous version did not take into account weight swapping.
Old test reimplemented parts of decoder and compared output of proc
against reimplementation. It had very little value (as we are likely to
slip same bug into both implementations) and was hard to reason about.

New test compares decoder against readable explicit golden prepared to
test all recently fixed bugs (swapped weights, bad description size
and wrong last weight position)
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
Co-authored-by: Wojciech Sipak <wsipak@antmicro.com>
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@magancarz
Copy link
Copy Markdown
Contributor Author

magancarz commented Mar 17, 2026

I've rebased changes on latest main with fixes from #3931 so the CI can pass.

@magancarz
Copy link
Copy Markdown
Contributor Author

I've updated Bazel and pip lock files.

@kbieganski
Copy link
Copy Markdown
Contributor

@proppy PTAL. We rebased earlier, but there are some new conflicts. We'd rather hold off on rebasing again for now until we get an initial round of feedback.

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.

7 participants