abs-tudelft / fletcher

Fletcher: A framework to integrate FPGA accelerators with Apache Arrow
https://abs-tudelft.github.io/fletcher/
Apache License 2.0
219 stars 31 forks source link

Add test with nullable field to demonstrate Fletchgen issue #213

Closed mbrobbel closed 1 year ago

mbrobbel commented 4 years ago

This reproduces the issue encountered by Mengfei when using nullable fields. It breaks the sum example when modified to have a nullable field.

This PR should not be merged but can be used to reproduce the issue.

mbrobbel commented 4 years ago
Elaborating simtop_tc...
Running simtop_tc...
/usr/local/bin/ghdl:error: bound check failure at /home/vsts/work/1/s/examples/nullable-sum/hardware/vhdl/Sum_ExampleBatch.gen.vhd:99
  from: work.sum_examplebatch(implementation).DEFAULT_CNFIG.arrayreader at Sum_ExampleBatch.gen.vhd:106
/usr/local/bin/ghdl:error: error during elaboration
johanpel commented 4 years ago

This is due to the config string generating an extra bit for validity bitmaps that are not present on the command stream to array readers. There is a test for the data channel widths vs. config string generated widths but not for the command channel widths vs. config string generated widths.

A fix would need to add this test and add the extra bits for any nullable types (potentially nested) as vhdmmio control field and properly connect it at the nucleus level. At the nucleus level, to merge vhdmmio's output (buffer addresses) and the kernel's command stream output (first and last index) a vhdl component is used called ArrayCmdCtrlMerger. This would need to be extended to allow for these bit to be merged onto the command stream coming from the mmio.

A work-around would be to correct the size of this channel and start implementing from the nucleus rather than the kernel level of abstraction.