Experimental bitstream compression support

[LAK132]

This PR adds an option to compress bitstreams. Tested with an XC7A200T(FBG484-2) (MEGA65 R3).

This is needed for using open tooling with the MEGA65 as the uncompressed bitstreams are too large to fit in the core slots.

With the cores I tested this with, the compression step didn't seem to have any noticeable impact on build times.

[mithro]

@kgugala - Could you take a look?

[kgugala]

@LAK132 thanks for the PR, It seems the CI is red. There is a lot compilation errors like:

03:00:22 | /root/prjxray/prjxray/lib/xilinx/tests/xc7series/configuration_test.cc:281:24: error: 'struct prjxray::xilinx::Configuration<prjxray::xilinx::Series7>::PacketData' has no member named 'size'
03:00:22 |   281 |  EXPECT_EQ(packet_data.size(), 15 * 101);

You mentioned that you had to change C++ standard. @LAK132 which standard did you use in your local build?

[LAK132]

I just moved the standard setting thing to the top of the script, it's still C++14. one of the dependencies didn't like that the standard wasn't being set before it was compiled, so I moved it.

I'll take a look at those errors, must be coming from something I didn't try to build

[kgugala]

looks like all the databases (artix, zynq, kintex and spartan) builds failed with:

ValueError: invalid literal for int() with base 16: '<0'

in the 005-tilegrid/clb fuzzer. Here is the whole trace:

Traceback (most recent call last):
File "add_tdb.py", line 162, in <module>
     main()
   File "add_tdb.py", line 158, in main
     run(args.fn_in, args.fn_out, verbose=args.verbose)
   File "add_tdb.py", line 138, in run
     for (tile, frame, wordidx) in load_db(tdb_fn):
   File "add_tdb.py", line 51, in load_db
     check_frames(tagstr, addrlist)
  File "add_tdb.py", line 21, in check_frames
     frame = parse_addr(addrstr, get_base_frame=True)
   File "add_tdb.py", line 30, in parse_addr
     frame = int(line[0], 16)
 ValueError: invalid literal for int() with base 16: '<0'

Since all the DB builds fail with the same error, I suspect the changes in the tools trigger this. @LAK132 can you check this?

[LAK132]

I'm struggling to get the environment set up for running these tests on my local machine. if it wasn't the fasm2frames changes then I haven't the slightest clue what could be happening

[mithro]

@LAK132 - If you are having issues getting the tool / testing environment setup, please log github issues and we will try and improve both the instructions and the process.

[hansfbaier]

fuzzers/005-tilegrid/bram_block/build_xc7a100tfgg676-1/segbits_tilegrid.tdb has no candidates entries:

[...]
BRAM_L_X44Y65.DWORD:0.DFRAME:00 00C00100_030_00
BRAM_L_X44Y70.DWORD:0.DFRAME:00 00C00100_040_00
BRAM_L_X44Y75.DWORD:0.DFRAME:00 00C00100_051_00
BRAM_L_X44Y80.DWORD:0.DFRAME:00 00C00100_061_00
BRAM_L_X44Y85.DWORD:0.DFRAME:00 00C00100_071_00
BRAM_L_X44Y90.DWORD:0.DFRAME:00 00C00100_081_00
BRAM_L_X44Y95.DWORD:0.DFRAME:00 00C00100_091_00
BRAM_L_X6Y0.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y10.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y100.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y105.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y110.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y115.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y120.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y125.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y130.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y135.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y140.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y145.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y15.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y150.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y155.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y160.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y165.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y170.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y175.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y180.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y185.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y190.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y195.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y20.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y25.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y30.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y35.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y40.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y45.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y5.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y50.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y55.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y60.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y65.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y70.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y75.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y80.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y85.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y90.DWORD:0.DFRAME:00 <0 candidates>
BRAM_L_X6Y95.DWORD:0.DFRAME:00 <0 candidates>
BRAM_R_X51Y100.DWORD:0.DFRAME:00 00800180_000_00
BRAM_R_X51Y105.DWORD:0.DFRAME:00 00800180_010_00
BRAM_R_X51Y110.DWORD:0.DFRAME:00 00800180_020_00
BRAM_R_X51Y115.DWORD:0.DFRAME:00 00800180_030_00
BRAM_R_X51Y120.DWORD:0.DFRAME:00 00800180_040_00
BRAM_R_X51Y125.DWORD:0.DFRAME:00 00800180_051_00
[...]

[hansfbaier]

I narrowed down the problem to the bitread tool: When I replace it with a known good version then the candidates are found.

[hansfbaier]

@LAK132 This change in configuration.h looks suspicious to me, it possibly has different semantics than the original code:

[hansfbaier]

@LAK132 Here are two example bitstreams which show the difference:
example-data.tar.gz
Consider the .bits files there (the output of bitread).
the -bad version (with the bitread tool from this branch) seems to have extraneous data (right hand side):
EDIT: I looks like I swapped the -good and -bad parts in the example. The bitread tool with the extra lines seems to be the "good" one.

So you might want to debug bitread with those input files.
I strongly suspect the changes in method Configuration<ArchType>::InitWithPackets(const typename ArchType::Part& part, Collection& packets) has something to do with that.

[LAK132]

it possibly has different semantics than the original code

Technically true. Previously if that address had already been written to it would overwrite it, whereas with .insert it will not overwrite existing frame writes. That said, writing to the same frame multiple times feels like a bug? The beauty of .insert is that it won't default construct a frame before the assignment can take place. That said, that change should have been the only major change to the uncompressed codepath, so if reverting that fixes the problem...

[LAK132]

Also it looks like with that example data, it's the -good output that has the extra data, not the -bad output? Were these outputs made with or without compression enabled?

[hansfbaier]

@LAK132 Sorry, I was wrong, the original bitread has the extra bits (green parts above), the bitread of this branch does not have them. So you are right, the -good output has the extra data.

[hansfbaier]

@LAK132 I think all the vivado runs in the fuzzers have no bitstream compression.

[hansfbaier]

@LAK132 Here is a 'good' version of bitread for comparison
bitread.gz

[hansfbaier]

@LAK132 So the way to go would be:

1. instrument the code with printfs to find out what happenes with the missing parts, or use a debugger
2. run ./build/tools/bitread -part_file part.yaml -F 0x00000000:0xffffffff -o design.bits -y -z design.bit (take design.bit from the example data, either the good or the bad version, they are equivalent)
   I attached the part.yaml for your convenience
   part.yaml.gz

[LAK132]

.insert_or_assign would have been a better way to do this but unfortunately that's a C++17 feature

[hansfbaier]

@LAK132 Did that fix the bitread results?

[LAK132]

It should have, the uncompressed codepath should be practically untouched now

[hansfbaier]

Can you test it? End of the day for me here...

[LAK132]

The bitread output appears to be the same as the known good version with my test core project. I still haven't managed to run any of the other tests myself.

[hansfbaier]

@LAK132 OK great let's wait for the CI then

[hansfbaier]

@LAK132 Good news, the changes work well here on my machine. I added a temporary PR (see below) to run the full CI on this build. @kgugala @mkurc-ant Can we run the CI on this?

[hansfbaier]

@LAK132 CI results look great! (Only the gtp-common-pips fuzzer is red, but that one is notoriously flickering) @mkurc-ant @kgugala Ready for review!

[hansfbaier]

@tmichalak Would it be possible to run the CI on this branch?

[mithro]

I have kicked off the CI and invited LAK132 to be a contributor so that the CI should run automatically from now on.

[LAK132]

Is there a reason the tests were cancelled? Did they just time out?

[kgugala]

Is there a reason the tests were cancelled? Did they just time out?

they should not timeout. I've restarted them

[LAK132]

All green now, anything else I need to do?

[LAK132]

Any update on this?