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

modules/zstd: Add support for decoding compressed blocks #1857

Open
wants to merge 85 commits into
base: main
Choose a base branch
from

Conversation

lpawelcz
Copy link
Contributor

@lpawelcz lpawelcz commented Jan 15, 2025

This PR extends the ZstdDecoder with support for decoding compressed blocks.

It supersedes PRs:

The decoder is capable of decoding RAW and RLE literals as well as sequences with predefined FSE tables.
A suite of DSLX tests comprising unit tests of all underlying procs and an integration test was prepared.
The integration test, similarly as in #1654, first generates a random valid ZSTD frame with compressed blocks and expected decoded output. Test data is then converted to a DSLX file (example) that is imported by the integration tests file.
At the beginning of the test, the default FSE decoding tables are filled with default distributions taken from RFC 8878 section 3.1.1.3.2.2. Default Distributions . Next, the encoded frame is loaded to the system memory and the decoder is configured through a set of CSRs to start the decoding process. The decoder starts the operation and writes the decoded frame back into the output buffer in the system memory. Once it finishes, it sends a pulse on the notify channel signaling the end of the decoding. The output of the decoder is compared against the decoding result from the reference library.

The PR introduces among others:

  • CompressedBlockDecoder - manages both SequenceDecoder and LiteralsDecoder to enable compress block decoding. Integrated with the top-level ZstdDecoder
  • SequenceDecoder - responsible for decoding sequence sections of the compressed blocks
  • FseDecoder - introduced as the core part of the SequenceDecoder
  • RefillingShiftBuffer - used for storing and outputting in forward and backward fashion an arbitrary amount of bits required by the FSE decoder
  • LiteralsDecoder - capable of decoding RAW, RLE and Huffman-coded literals
  • HuffmanDecoder - used in decoding huffman-coded literals. Decoded Huffman trees are then used to decode one or four Huffman-coded streams.
  • CommandConstructor - this proc is responsible for sending packets with decoded sequences and literals to the SequenceExecutor proc
  • RamMux and RamDemux - procs used for handling requests/responses to multiple memory models. The procs interface with 3 separate memory buffers for FSE decoding tables.

Copy link

google-cla bot commented Jan 15, 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
Member

proppy commented Jan 27, 2025

Can we rebase and consolidate with #1654 ?

@lpawelcz
Copy link
Contributor Author

lpawelcz commented Feb 5, 2025

Can we rebase and consolidate with #1654 ?

@proppy done
This PR also supersedes #1616 so I also closed it in favor of this one

@@ -59,6 +59,11 @@ xls_dslx_prove_quickcheck_test(
srcs = ["std.x"],
)

xls_dslx_library(
Copy link
Member

Choose a reason for hiding this comment

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

can we get a separate PR for that?

@@ -0,0 +1,170 @@
# Copyright 2024 The XLS Authors
Copy link
Member

@proppy proppy Mar 6, 2025

Choose a reason for hiding this comment

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

can. you add a docstring about what the module is doing and where it is being used?

@@ -0,0 +1,33 @@
# Copyright 2024 The XLS Authors
Copy link
Member

Choose a reason for hiding this comment

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

can. you add a docstring about what those external are and why they are needed?

@@ -24,6 +26,15 @@ pub struct PlainData<SYMB_WIDTH: u32> {
last: bool, // flush RLE
}

// Structure contains multiple uncompressed symbols.
Copy link
Member

Choose a reason for hiding this comment

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

is that used by zstd?

@@ -0,0 +1,753 @@
// Copyright 2024 The XLS Authors
Copy link
Member

Choose a reason for hiding this comment

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

can. you add a docstring about what the module is and why it is needed? (if it's for driving the cocostb test maybe move it there or in a separate rtl folder?)

@@ -0,0 +1,193 @@
// Copyright 2024 The XLS Authors
Copy link
Member

Choose a reason for hiding this comment

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

can you move the verilog file in a separate rtl directory w/ a README (or docstring describing the modules)?

@proppy
Copy link
Member

proppy commented Mar 6, 2025

can you also rebase?

koblonczek and others added 12 commits March 26, 2025 22:59
- 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

Internal-tag: [#64075]
Signed-off-by: Krzysztof Obłonczek <[email protected]>
Co-authred-by: Pawel Czarnecki <[email protected]>
Co-authred-by: Robert Winkler <[email protected]>
Signed-off-by: Michal Czyz <[email protected]>
Signed-off-by: Pawel Czarnecki <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
rw1nkler and others added 29 commits March 26, 2025 23:07
Co-authored-by: Robert Winkler <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Signed-off-by: Krzysztof Obłonczek <[email protected]>
Co-authored-by: Robert Winkler <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Signed-off-by: Krzysztof Obłonczek <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Co-authored-by: Krzysztof Obłonczek <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
Internal-tag: [#71874]
Signed-off-by: Pawel Czarnecki <[email protected]>
Internal-tag: [#71874]
Signed-off-by: Pawel Czarnecki <[email protected]>
Internal-tag: [#71766]
Signed-off-by: Pawel Czarnecki <[email protected]>
Co-authored-by: Krzysztof Obłonczek <[email protected]>
Signed-off-by: Maciej Torhan <[email protected]>
Signed-off-by: Robert Winkler <[email protected]>
@rw1nkler rw1nkler force-pushed the zstd_compressed_block_dec branch from 16cd455 to cae18b3 Compare March 27, 2025 06:21
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