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.

Sign up for GitHub

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

Jump to bottom

modules/dbe - Lz4 decoder #1075

Closed
wants to merge 7 commits into from
Closed

modules/dbe - Lz4 decoder #1075

wants to merge 7 commits into from

Conversation

rdob-ant
Copy link
Contributor

This is an implementation of Lz4 decoder in DSLX and accompanying cocotb testsuite. Since Lz4 encoder reuses much of the code from this PR, I'd like to prepare it after this one is reviewed.

Some notes:

  1. This relies on cocotb, so includes commits from Add cocotb framework #1057 . Once that is merged, this branch will be rebased.
  2. DSLX implementation of decoder uses 1R1W RAM with 1 partition. Such configuration triggers a small bug (missing mask expansion) in examples/ram.x that this PR fixes.
  3. It also includes reference decoder implementation in Python which is not used by the tests in the current PR, but will be needed for the upcoming Lz4 encoder PR.

rw1nkler and others added 7 commits July 21, 2023 17:50
@rw1nkler @rdob-ant
FIXME: Use cocotb_test() from forked repository
fb77645 
Temporary get improved version of the cocotb_test() rule from
a forked repository.

Internal-tag: [#44586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler @rdob-ant
deps: Add cocotb and cocotb_bus to python requirements
5d8d502 
Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler @rdob-ant
Add cocotb python library for XSL
a17fc75 
The library contains the XLSChannelDriver and XLSChannelMonitor classes,
which can be used to send or receive data from an XLS channel in
a Cocotb simulation, respectively.

Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler @rdob-ant
Add a simple cocotb example
977ab00 
This commit adds an example that uses the Cocotb framework to validate
behavior of the Verilog file converted from a simple DSLX code

Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rw1nkler @rdob-ant
Add cocotb_xls_test macro
65cec5e 
Creates cocotb_xls_test macro that generates a file with the
timestamp specified in the rule definition. The macro is required to
generate the timestamp for Icarus Verilog smulation in Cocotb.

Internal-tag: [#46586]
Signed-off-by: Robert Winkler <rwinkler@antmicro.com>
@rdob-ant
examples/ram: 1R1W - fix missing mask expansion
a29cb5d 
Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>
@rdob-ant
modules/dbe: add lz4 decoder
836e566 
Decoder for Lz4 - a dictionary-based data compression algorithm.

Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>
@rw1nkler rw1nkler mentioned this pull request Jul 21, 2023
Closed
@proppy proppy mentioned this pull request Jul 23, 2023
Closed
6 tasks
@proppy
Copy link
Member

proppy commented Jul 23, 2023

Related to #995.

@proppy
Copy link
Member

proppy commented Jul 24, 2023

Such configuration triggers a small bug (missing mask expansion) in examples/ram.x that this PR fixes.

can we get a separate PR for that (so that we can merge it earlier)

@proppy
Copy link
Member

proppy commented Jul 24, 2023

This relies on cocotb, so includes commits from #1057 . Once that is merged, this branch will be rebased.

Maybe we could move this to a separate PR, as it is not strictly required by the implementation (and not to block this being merged on the resolution of the discussions happening in #1057)

@proppy
Copy link
Member

proppy commented Jul 24, 2023

It also includes reference decoder implementation in Python which is not used by the tests in the current PR, but will be needed for the upcoming Lz4 encoder PR.

Do you plan to validate the dslx/ir/codegen versions against the python reference impl?

@proppy
Copy link
Member

proppy commented Jul 24, 2023

Lz4 encoder reuses much of the code from this PR, I'd like to prepare it after this one is reviewed.

I'd be good to get the encoder and the decoder in the same PR, in order to be able to more effectively cross-review them. If it's challenging to send both at the same time, I think it'd be easier to review the encoder first.

@rdob-ant
Copy link
Contributor Author

can we get a separate PR for that (so that we can merge it earlier)

Opened #1078

@rdob-ant
Copy link
Contributor Author

Maybe we could move this to a separate PR, as it is not strictly required by the implementation (and not to block this being merged on the resolution of the discussions happening in #1057)

We can, but since I wrote most of the tests in cocotb, I'm afraid without them the implementation may look "incomplete".

@rdob-ant
Copy link
Contributor Author

Do you plan to validate the dslx/ir/codegen versions against the python reference impl?

Yes, at least in one test I'm using Python decoder to decode (and thus validate) the output of DSLX encoder.

@rdob-ant rdob-ant mentioned this pull request Jul 25, 2023
Closed
@rdob-ant
Copy link
Contributor Author

Superseded by #1079 that includes both encoder and decoder.

@rdob-ant rdob-ant closed this Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rdob-ant @proppy @rw1nkler