modules/dbe - lz4 encoder and decoder [DSLX code only] #1092

rdob-ant · 2023-08-03T13:28:00Z

This is an implementation of Lz4 encoder and decoder in DSLX without reference Python & cocotb test code that was present in #1079.

The source branch additionally includes changes from #1078 that are needed to run DSLX tests of procs using 1R1W single-partition RAM. These will be dropped by rebasing when we get that PR merged.

tmichalak · 2023-08-03T13:31:37Z

CC @proppy @hongted

hongted · 2023-08-08T01:21:25Z

xls/examples/ram.x

-        std::convert_to_bools_lsb0(read_req.mask))
+      let init_parts = mem_initialized[read_req.addr];
+      let requested_is_init = for (idx, requested_is_init): (u32, bool) in
+      range(u32:0, NUM_PARTITIONS) {


Add 1 level of indentation.

Hi @hongted . Sure, will do that. To keep things clean, I propose to discuss ram.x changes in a PR dedicated to that bugfix: #1078 . I will address your comments directly there.

This change is present in the current PR only because Lz4 DSLX tests depends on the RAM bugfix, and I'm not aware of a way of submitting Lz4 changes for review without making commits from a dependency PR visible here.

hongted · 2023-08-08T01:30:46Z

xls/examples/ram.x

+      let requested_is_init = for (idx, requested_is_init): (u32, bool) in
+      range(u32:0, NUM_PARTITIONS) {
+        if read_req.mask[idx+:bool] {
+          if !init_parts[idx] {
+            trace_fmt!("reading partitions {} but part={} is not init: {}",
+              std::convert_to_bools_lsb0(read_req.mask), idx, init_parts);
+          } else { () };
+          requested_is_init && init_parts[idx]
+        } else { requested_is_init }
+      } (true);


This seems general enough to suggest a separate function and unit test.

Also it seems that this can be simplified to just
let mem_initialized_as_bits = std::convert_to_bits_lsb0(mem_initialized[req_req.addr]);

assert_eq(mem_initialized_as_bits, mem_initialized_as_bits | read_req.mask)

Fixed in #1078

hongted

Can a round-trip test be added?

i.e.

Create a round-trip test proc that spawns an encoder and decoder back to back.
Give it some arbitrary plain text and assert that what goes in comes out.

hongted · 2023-08-08T01:40:08Z

xls/modules/dbe/BUILD

+    name = "dbe_common_dslx",
+    srcs = [
+        "common.x",
+        "common_test.x"


Move common_test into a xls_dslx_test rule.

Hm.. The constructs from common_test.x are imported in both lz4_decoder.x and lz4_encoder.x (since the test procs are defined in the same files where the real procs are implemented, which looks like a common pattern in DSLX). All imports must be resolvable at all times, not only when running DSLX tests. Without moving those tests to separate files, I don't think it's possible to factor out common_test.x and import it only when running the tests 🤔

Personally I'd factor out the tests into their own dslx file -- that keeps the interface cleaner.

Will do in the upcoming update

I've factored out the tests & added a round-trip test.

hongted

https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md a TODO?

hongted · 2023-08-09T14:58:08Z

xls/modules/dbe/BUILD

+    name = "dbe_lz4_decoder_opt_ir",
+    src = "dbe_lz4_decoder_ir.ir",
+    top = "__lz4_decoder__decoder__decoder_base_0__16_16_8_next",
+)


As the dslx is using an abstract ram model, suggest adding the transforms to a real ran model in both the opt_ir and then in codegen -- following https://github.com/google/xls/blob/main/xls/contrib/xlscc/examples/BUILD#L152

Sure, I will look into this.

@hongted Hm, I'm a bit confused here. I see that code that targets XLScc (which I haven't looked into at all till today) indeed represents RAM as an abstract object with array-like semantics, which has to be then rewritten to map onto a real-world 1RW or 1R1W RAM core.

Contrary to that, my DSLX code, as well as existing DSLX example code in e.g. xls/examples/delay.x is written with a specific RAM core in mind from the beginning - there is a protocol for 1R1W core (4 channels: 2x output for rd & wr requests, 2x input for rd & wr responses) and a protocol for 1RW core (2 channels: 1x output for combined rd or wr request, 1x input for combined rd or wr response).

If there a way to write DSLX code to target 'abstract' RAM and then rewrite it at opt_ir stage like in case of XLScc, do you know if there's example code that shows this, or at least some file with DSLX struct definitions for the abstract protocol?

While the example is XLSCC, both XLSCC and DSLX are translated to a common IR.

The idea here is to use to target an an abstract ram model so that we can easily retarget the implementation to whatever ram is available. Whether it's 1RW, 1R1W, etc...

Forgot to reply: this was addressed, now there's a configuration for IR optimizer to rewrite the RAM interface.

hongted · 2023-08-09T14:59:33Z

xls/modules/dbe/BUILD

+        "reset": "rst",
+        "use_system_verilog": "false",
+        "streaming_channel_data_suffix": "_data",
+        "io_constraints": ",".join([


Following above comment, add an additional ram_config -- per https://github.com/google/xls/blob/main/xls/contrib/xlscc/examples/BUILD#L273

This enables switchting between different RAM specifications more easily

From what I understood, I'll first have to modify DSLX proc to use the abstract RAM protocol (see my reply above). Please let me know if got it wrong.

Correct -- it should be a minimal change.

hongted · 2023-08-09T15:00:34Z

xls/modules/dbe/common.x

+    MK = 2, // Control marker
+}
+
+pub struct Token<SYM_WIDTH: u32, PTR_WIDTH: u32, CNT_WIDTH: u32> {


This doesn't seem to follow the LZ4 spec -- https://github.com/lz4/lz4/blob/dev/doc/lz4_Block_format.md

This is an "internal" token object used by encoder & decoder algorithms. There are other procs (not included in this PR) needed to convert between a stream of these _Token_s and the actual sequence/block format used by Lz4 files.

I chose such design because Lz4 block is not just a sequence of flattened & serialized tokens - to optimize file footprint they perform grouping of LITERAL tokens and have a special way of encoding COPY_POINTERs (e.g. part of CP_CNT is stored in the first byte of the sequence, and the rest gets appended at the very end of the sequence, for example). While encoder & decoder algorithms shouldn't really deal with such specifics of file encoding, they only care about raw LITERAL and COPY_POINTER tokens.

Understood. Add a TODO if this the plan is to address the comment in a follow up change.

However, I think the current design should be such that it's possible to implement it in the future with minimal changes. One thing I don't see here is how to handle the variable literal lengths available in LZ4.

What is the plan to address that?

As for variable literal lengths in LZ4 block format.

A single Token represents either a single literal (symbol), or a single match (copy) operation. If I understand it right, the "variable literal length" that you're referring to is a way to specify the number of literals present in LZ4 sequence, where a single LZ4 sequence is defined as N literals followed by a single match operation (except for the last sequence in the block, which has only literals and no match operation). Important detail of the format is that you have to first specify the number of literals that the sequence contains (using a variable number of bytes that depends on how many literals do you have), and only then specify the literals themselves. The extreme case is of an LZ4 block that contains only literals, without any match operation - so the number of literals in the sequence approx. corresponds to the block size, which iirc can be as large as 4MB.

W.r.t. hardware implementation, LZ4 algorithm itself can be laid out rather nicely as it's stream-oriented and doesn't need any extra buffering besides 64KB history buffer and 8~32KB hash table.

However, if one is to implement an IP core that emits data in a standard LZ4 block format, due to the fact that LZ4 compressor emits tokens sequentially and their type and count is not known beforehand, and the fact that the LZ4 block specifies number of literals before the literals themselves, they'll first need to be buffered (with buffer size limited by the max block size), and only then one will be able to emit an LZ4 sequence header and then emit literals themselves. It all can be done within a single buffer as well, but you'll still need a block buffer :)

There are alternative encoding formats that try to mitigate this issue e.g. MLZ4, but they are not compatible with existing LZ4 software.

In the scope of this PR, the idea was to implement an LZ4 encoder core that will accept a stream of raw bytes on the input, and output a stream of tokens: literals or matches, without targeting a specific block encoding format. Packing of tokens into blocks (using LZ4 or MLZ4 or any other format) is thus offloaded to an external proc, which is out of scope of this PR. However, the encoder core should of course emit enough data to allow such packing - and that's why I've recently added e.g. a "MATCHED_SYMBOL" token to allow easily rewriting matches (needed to address some "special" LZ4 requirements w.r.t. minimal match length and minimal literal sequence length at the end of the block).

I think my concern here is that it is not possible to output the Lz4 block encoding from this token stream. As you mention, an implementation would need store the entire token stream, and then process that.

Sounds like what you are saying is that all implementations have that limit? If so it sounds like LZ4 isn't a suitable compression algorithm for streaming hardware acceleration.

@proppy -- thoughts?

The algorithm itself is stream-oriented and plays rather well with the HW, but the encoding of tokens into byte sequences does not. You can check e.g. the aforementioned MLZ4 in part of encoding - they've modified it a bit so that it's more hardware-friendly and does not require excessive buffering. They've also improved the algorithm itself a bit, but that is not significant in this context.

hongted · 2023-08-09T15:02:34Z

xls/modules/dbe/common.x

+}
+
+pub enum TokenKind : u2 {
+    LT = 0, // Literal


Instead of a comment, just have TokenKind be the unexpanded name ie LITERAL instead of LT.

This is a common comment. While there is no formal DSLX standard yet, we prefer to have longer more meaningful names. Refer to https://google.github.io/styleguide/cppguide.html#General_Naming_Rules

hongted · 2023-08-09T15:03:11Z

xls/modules/dbe/common.x

+    mark: Mark
+}
+
+pub fn zero_token<SYM_WIDTH: u32, PTR_WIDTH: u32, CNT_WIDTH: u32>()


Suggest using the builtin zero!<>. This is a common comment.

hongted · 2023-08-09T15:08:44Z

xls/modules/dbe/lz4_decoder.x

+        // Check whether rd_ptr points to an unwritten HB cell
+        let rd_ptr_valid = (rd_ptr < state.wr_ptr) || state.hb_all_valid;
+
+        // HB READ - read the symbol from history buffer


Does this handle the "overlap match" scenario referenced in https://github.com/lz4/lz4/blob/dev/doc/lz4_Block_format.md when the symbol from the history buffer depends on the match copy?

Yes, there were cocotb tests in the original PR that cover this, e.g. https://github.com/antmicro/xls/blob/6b324c35883898f77933c7355eb1d7f5daee6339/xls/modules/dbe/lz4_decoder_test.py#L171

I'd leave this to @proppy to decide whether or not the DSLX tests should be more comprehensive.

hongted · 2023-08-09T15:10:29Z

xls/modules/dbe/lz4_decoder.x

+        // HB WRITE - write emitted symbol to the history buffer
+        let do_hb_write = data_valid && !data.is_mark;
+        let tok = send_if(tok, o_ram_hb_wr_req, do_hb_write,
+            ram::WriteWordReq<u32:1>(state.wr_ptr, data.data));


LZ4 is a byte-oriented compression right? Should there be some use of masks to handle a WordReq'ed sized write?

It is. This RAM's data bus has width of a single symbol, so the "word size" is 8 bits in case of a standard LZ4, masks are not needed.

I think a TODO is warranted here then. I think most RAMs we'll have available for implementation would have a data bus of greater than 8 bits.

I can add a TODO. But do we want to make DSLX code aware of the port size of a specific RAM IP used in the hardware? E.g. in FPGAs you'd frequently rely on RAM block being auto-inferred from the Verilog code, thus the synthesis tool takes care of a mismatch between the data/address bus widths of available hard-IP core and the expectations set by the RTL design.

I think it should be an input parameter. SRAMS take a non-trivial amount of time to generate collateral and incorporate during an ASIC desgin flow.

hongted · 2023-08-09T15:13:12Z

xls/modules/dbe/lz4_decoder.x

+    hb_all_valid: bool,
+}
+
+pub proc decoder_base<SYM_WIDTH: u32, PTR_WIDTH: u32, CNT_WIDTH: u32> {


In terms of general architecture, consider how to support the following

Using single port RAM

Whether multiple procs accessing a single RAM could make the behavior clearer/cleaner.

I'm not sure what's the policy w.r.t single/dual-port RAMs.

In case we'd always prefer to use a single-port RAM, then this block can be rewritten to do that.

In case we want to have both options available, then a proc that uses 1R1W RAM is more versatile, as it can either use 1R1W directly, or it can be bridged to a 1RW RAM via some kind of 1R1W-1RW adapter proc (this last claim needs some thinking though.. I reckon there may be some corner cases where this won't work).

See comment above -- if abstract channels are used then RAM architecture comes free.

However, #2 remains. Would it make sense to break apart the proc into separate communicating procs using the same SRAM?

Hm, even if we're just considering splitting the current monolithic LZ4 compressor into multiple procs, having a few procs communicating with the same RAM may make things easier. But is this already supported in DSLX? Can you perhaps point me to such an example? I thought that a single RAM can be used at most by two procs (one reading, one writing) 🤔

Yes -- I don't think we've contemplated more than two procs for an SRAM. But it's possible to write a SRAM-interface proc that will handle the arbitration.

hongted · 2023-08-09T15:14:26Z

xls/modules/dbe/lz4_decoder.x

+
+        // Send decoded data symbol or Mark
+        let zero_data = dbe::zero_plain_data<SYM_WIDTH>();
+        let (data_valid, data) = if rx_reset {


rx_rest is more about clearing the state to handle a new block, right? If so, consider changing the name to reflect that.

In general it's an equivalent to a complete state reset: it both prepares the proc to handle a new block of data, as well as it clears a sticky error state that the proc (and other procs in the chain) may be in.

It seems the common case is to handle the new block of data, no? I'd suggest naming it that rather than the less common case of resetting the error state.

For now I've kept the "reset" name, as the idea of "RESET" token was to reset a full network of processing blocks (e.g. LZ4 block encoder or anything else that can be there besides this single core), and it just happened that in the encoder this function coincides with the operation that has to be done when you want to start processing of a new independent block. I can rename it to "restart" or something like that, or even add a new marker code, but that would be redundant (in a sense that if you're starting to encode an independent block of data, you don't want to preserve any previous state, so why not just completely reset the circuit?).

I'd say the issue here is that the reset signal is usually special -- and may be considered a multi-cycle signal.

It sounds like this signal here is expected to be a mission-mode signal that should be timed as such -- so it's more a personal preference?

hongted · 2023-08-09T15:21:08Z

xls/modules/dbe/lz4_decoder.x

+];
+
+#[test_proc]
+proc test_simple {


Suggest adding a couple of more tests to target edge conditions

"overlap match"

History buffer wraparouond

offset zero

These tests, except for "offset zero" (as that belongs to the lz4_block_reader proc that parses Lz4 block format and is not part of the current PR) are implemented using Python Cocotb framework, they were part of the original large PR #1079:

https://github.com/antmicro/xls/blob/6b324c35883898f77933c7355eb1d7f5daee6339/xls/modules/dbe/lz4_decoder_test.py

hongted · 2023-08-09T15:24:27Z

xls/modules/dbe/lz4_encoder.x

+    }
+}
+
+pub proc encoder_base<


Add a comment on how matches are found using the hash table (i.e. algorithm).
Comment on how aliases in hashing are found and prevented from causing a bad encoding.

I'm going to prepare a document that explains how the algorithm works - this will be explained there.

@rdob-ant curious which format do you plan to use? (separate markdown file, comments in the header of the impl, or google docs?)

@proppy I wrote a plain markdown file.

cdleary · 2023-08-13T05:03:10Z

xls/modules/dbe/lz4_encoder.x

+            FsmSt::ERROR
+        } else if rx_reset {
+            FsmSt::RESET
+        } else if fsm == FsmSt::RESET {


Nit: usually we prefer a match statement for FSMs in the DSL (so you can demonstrate that you exhaustively handled all cases in a declarative fashion).

proppy · 2023-08-24T14:47:07Z

xls/examples/ram.x

Looks like there is a conflict, can we rebase?

rdob-ant · 2023-08-24T15:22:53Z

@proppy will rebase when pushing updated code

hongted · 2023-09-06T22:31:17Z

xls/modules/dbe/docs/README.md

@@ -0,0 +1,289 @@
+# Dictionary coder implementation in DSLX


Couple of generic comments

Given the long FSM backedge -- what's the expected throughput of this implementation?

Is the throughput/area tunable?

Perhaps a multi-proc implementation would be preferred to improve this.

As this is an example code, we haven't really considered the throughput while writing this code. However, as far as I remember, when I was simulating the generated Verilog code, I got something in the order of 10~12 clock cycles for a single byte (with 1-cycle single-port RAMs). Even a dual-port RAM will make it somewhat better, the number is still not good enough for a practical implementation that should be targeting something like 1 byte per cycle on average (at most).

Not directly. We can tune the area/compression ratio by changing the hash table size, and also using single-port -vs- double-port RAM or RAMs with different latency will result in different throughput/area ratios, but there is no parameter to control this directly in a smooth way (and tbh I can't think of any way this can be done in a scalar LZ4).

Probably yes. I don't see how it can be improved without splitting the design.

hongted · 2023-09-06T22:34:42Z

xls/modules/dbe/docs/README.md

+* *data* communicates a symbol whenever *is_marker* is not set.
+* *mark* communicates a control mark whenever *is_marker* is set.
+
+#### Token


I'd suggest writing another section on how these tokens can be converted into a stream compatible with the LZ4 block format (https://github.com/lz4/lz4/blob/dev/doc/lz4_Block_format.md)

Notably the following constraints

That there is a minimum match length of 4 bytes

That after each literal section there MUST be a minimum match

The end of the sequence must have 5 literal bytes

The last match must have an offset >= 12

I will do this. In short, the idea is that you can use data conveyed within "MATCHED_SYMBOLs" to rewrite "MATCHes", e.g. if your encoding format requires a match of at least 4 symbols, and the encoder emits a match of 2 symbols, there will be two MATCHED_SYMBOL tokens accompanying a MATCH, so the postprocessor can remove the match and convert MATCHED_SYMBOLs into UNMATCHES_SYMBOLs (i.e. literals).

This way it is possible to remove or shorten matches, and I think it should be enough to cover all the requirements imposed by LZ4 standard.

Thanks -- I think the discussion on token format is relevant here as well. Is it even possible to do such a thing without needing to buffer the entire block?

I've added a description to the doc that shows how matches can be rewritten to fulfill these requirements.

One note, though, about requirement 2: That after each literal section there MUST be a minimum match - a match must be there, except if it's the end of the block. The end of the block must contain only literals and no match. Thus, a typical LZ4 block looks like this:

literals - match - literals - match - literals - END

One other requirement is about the minimal match size, and that's easy to implement with only 4 (or even 3) bytes of FIFO memory. The rest of requirements discuss how exactly that last sequence of "literals" has to look like. Those requirements can be fulfilled by making sure that there is no match within the last 12 symbols in the block - which is as simple as "buffering up to 12 symbols and if there are any matches in them, replacing those matches with literals". The tricky part is if the block ends in a long match that produces more than those 12 symbols. In this case, the match has to be split - and it's possible to do this without needing more than a 12-symbol buffer (or, how the doc says, 15-token buffer, since in reality we also want to buffer match tokens, there can be up to 3 of those per 12 symbols).

hongted · 2023-09-06T22:56:55Z

xls/modules/dbe/docs/README.md

+  grow, try to start a new matching string:
+  * __(7)__ Compute hash of the data contained in the FIFO.
+  * __(8)__ Load *candidate_ptr* pointer from the HT.
+  * __(9)__ Store *current_ptr* to the HT.


Seems the hash table is only updated when starting a new match, so that even assuming no collisions -- not all sequences will be located in the hash table?

That is during a match, the intermediate sequences will not be updated with the most recent offset.

Is this intended and matching the reference implementation?

I'll check this, but IIRC this behavior matches the reference code 🤔

@hongted See this line in Rust Lz4 implementation: https://github.com/PSeitz/lz4_flex/blob/main/src/block/compress.rs#L488

The code above this line looks for the beginning of the match and uses hash table to do that (lines 423~425). When the initial matching sequence of 4 bytes has been found, however, it tries to grow it (line 488) but this time without updating the hash table.

Example implementation of Lz4 - a dictionary-based data compression algorithm. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

Switches Lz4 encoder proc to use abstract (1R1W) RamModel and adds RAM interface rewrites at xls_ir_opt_ir and xls_ir_verilog stages. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

Make SimultaneousReadWriteBehavior enum public as it can be used from other modules. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

- Simplify Lz4 encoder by removing corner case handling required by Lz4 block format. This allows to remove 1 FIFO, 2 FSM states, and a bunch of convoluted logic that can be easily moved to an external module. - Hash function is factored out into a separate function - Variable naming is improved Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

Add a Markdown readme file and a couple of diagrams that explain how the encoder works. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

Factored out the tests into separate DSLX files. Added a round-trip test for encoder & decoder. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

Add information on match token rewriting. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

proppy · 2024-04-03T08:19:28Z

Should we close this in favor of the work on #1211 ?

rdob-ant mentioned this pull request Aug 3, 2023

modules/dbe - lz4 encoder and decoder #1079

Closed

rdob-ant force-pushed the 47181-lz4-dslx branch from 2425df2 to 6cc7050 Compare August 7, 2023 13:41

hongted approved these changes Aug 8, 2023

View reviewed changes

hongted requested changes Aug 8, 2023

View reviewed changes

rdob-ant mentioned this pull request Aug 9, 2023

bugfix: examples/ram: 1R1W - fix missing mask expansion #1078

Merged

hongted requested changes Aug 9, 2023

View reviewed changes

hongted reviewed Aug 9, 2023

View reviewed changes

cdleary reviewed Aug 13, 2023

View reviewed changes

rdob-ant force-pushed the 47181-lz4-dslx branch from 6cc7050 to d605867 Compare August 21, 2023 16:37

rdob-ant force-pushed the 47181-lz4-dslx branch 2 times, most recently from c20d324 to c7992e1 Compare August 29, 2023 18:35

rdob-ant force-pushed the 47181-lz4-dslx branch 2 times, most recently from 6d42979 to 2f30860 Compare September 1, 2023 19:00

hongted reviewed Sep 6, 2023

View reviewed changes

rdob-ant added 7 commits September 12, 2023 15:50

modules/dbe: add lz4 encoder and decoder

ce041d7

Example implementation of Lz4 - a dictionary-based data compression algorithm. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

modules/dbe: Lz4 encoder - use abstract RAM interface

d7e8145

Switches Lz4 encoder proc to use abstract (1R1W) RamModel and adds RAM interface rewrites at xls_ir_opt_ir and xls_ir_verilog stages. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

examples/ram: make SimultaneousReadWriteBehavior public

becfb46

Make SimultaneousReadWriteBehavior enum public as it can be used from other modules. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

modules/dbe: add documentation for LZ4 encoder

ac43f07

Add a Markdown readme file and a couple of diagrams that explain how the encoder works. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

modules/dbe: add round-trip test, refactor tests

cbce752

Factored out the tests into separate DSLX files. Added a round-trip test for encoder & decoder. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

modules/dbe: update docs

9e02abb

Add information on match token rewriting. Signed-off-by: Roman Dobrodii <rdobrodii@antmicro.com>

rdob-ant force-pushed the 47181-lz4-dslx branch from d49e9c6 to 9e02abb Compare September 12, 2023 18:46

cdleary added the app Application level functionality (examples, uses of XLS stack) label Mar 27, 2024

modules/dbe - lz4 encoder and decoder [DSLX code only] #1092

Are you sure you want to change the base?

modules/dbe - lz4 encoder and decoder [DSLX code only] #1092

Conversation

rdob-ant commented Aug 3, 2023

tmichalak commented Aug 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongted left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongted left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rdob-ant Sep 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy commented Aug 24, 2023

rdob-ant commented Aug 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongted Sep 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

proppy commented Apr 3, 2024 • edited

rdob-ant Sep 7, 2023 •

edited

hongted Sep 6, 2023 •

edited

proppy commented Apr 3, 2024 •

edited