Skip to content

Conversation

rdob-ant
Copy link
Contributor

@rdob-ant rdob-ant commented Jun 20, 2023

This is a draft PR that contains most of work done on dictionary-based algorithm so far. As a draft PR, this is not supposed to be reviewed in full, but rather serve as a material for discussions.

The status as of now is:

  • Generic DBE token type hierarchy in Python and DSLX
  • Python: universal decoder
  • Python: LZ4 encoder implementation with compression ratio estimator & basic correctness test
  • DSLX: universal decoder with simple test. TBD: history buffer array must be replaced by RAM for it to be practical.
  • DSLX: LZ4 encoder - basic port done, no tests yet. TBD: leads to SIGSEGV in IR converter.
  • DSLX: LZ4 framer and deframer - not started yet.

Signed-off-by: Roman Dobrodii <[email protected]>
@google-cla
Copy link

google-cla bot commented Jun 20, 2023

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.

)
}

next (tok: token, cur: State) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@proppy

I think this is a good example of why #1037 is needed.

What I'm trying to do here:

  • In some FSM states (e.g. START_MATCH_0), HB RAM is written (line 194), and then HT RAM is read (line 234).
  • In other FSM states (e.g. START_MATCH_1), HT RAM is written (line 245), and then HB RAM is read (line 262).

It's easy to prove that above states are mutually exclusive. However, I still can not use two mutually exclusive send's or recv's to the same channel. Replacing them with send_if / recv_if manually is not enough: the current XLS limitation seems to be that I must provide a single send_if or recv_if for each channel. There must not be two mutually exclusive send_if's, there must be at most one send_if in the whole next() function for each channel. While it is possible to rewrite the code to fulfill that constraint, the code most probably will get quite cumbersome and dirty as a result..

I see another option: instead of relying on 1RW memory, I should be able to use 1R1W memories here, and because those have separate channels for read and write requests, I may be able to figure out something that makes XLS happy without much code reshuffling.

Do you know what's the status of 1R1W RAM Verilog codegen support? From XLS soruces it seems that it may be supported, but I haven't found any 1R1W DSLX example that actually generates Verilog.

@proppy
Copy link
Member

proppy commented Jun 20, 2023

leads to SIGSEGV in IR converter.

is there a bug already filed about this?

@proppy
Copy link
Member

proppy commented Jun 21, 2023

Thanks for including the python reference implementation!

It would be interesting to including as a https://bazel.build/reference/be/python#py_test rule that test the its output against the dslx implementation, similar to what we have in C++ for some examples and modules, see:
https://github.com/google/xls/tree/main/xls/modules/aes
https://github.com/google/xls/blob/main/third_party/xls_colors/test.cc

There are existing private python bindings to the DSLX and IR interpreter that could be leveraged for doing this:

Note: there is also an embryo of a public python API there https://github.com/google/xls/tree/main/xls/public/python but it's currently very limited and is more targeted toward external usecases, rather than testing module that are maintained in the tree.

@tmichalak
Copy link

leads to SIGSEGV in IR converter.

is there a bug already filed about this?

Filed in #1035

@proppy
Copy link
Member

proppy commented Jul 23, 2023

is this superseeded by #1075?

@rdob-ant
Copy link
Contributor Author

@proppy Correct. Closing this one.

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