UCLA-VAST / tapa

TAPA is a dataflow HLS framework that features fast compilation, expressive programming model and generates high-frequency FPGA accelerators.
https://tapa.rtfd.io
MIT License
144 stars 27 forks source link

async_mmap streams destructed with leftovers #117

Closed vkomenda closed 1 year ago

vkomenda commented 1 year ago

Simulations of examples using async_mmap produce warnings about streams being non-empty at the time of destruction. I believe this is only because of the eot token which is still not consumed. For example, nested-vadd:

$ ./vadd 
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0921 12:13:32.443557 862266 task.h:54] running software simulation with TAPA library
W0921 12:13:32.986137 862266 stream.h:56] channel 'c' destructed with leftovers; hardware behavior may be unexpected in consecutive invocations
W0921 12:13:32.986152 862266 stream.h:56] channel 'b' destructed with leftovers; hardware behavior may be unexpected in consecutive invocations
W0921 12:13:32.986155 862266 stream.h:56] channel 'a' destructed with leftovers; hardware behavior may be unexpected in consecutive invocations
kernel time: 0.542527 s
PASS!

Shouldn't the examples be modified to eliminate these warnings? Do the eot tokens need to be consumed before the kernel finishes?

Blaok commented 1 year ago

Shouldn't the examples be modified to eliminate these warnings?

Ideally, yes, but it'll be of low priority. PRs are much appreciated :)

Do the eot tokens need to be consumed before the kernel finishes?

It is recommended, but not strictly required. The only problem is that those leftover tokens (eot or not) will still be there next time you launch the kernel without an explicit reset. If your program assumes that all streams are empty at the beginning, there might be problems.

vkomenda commented 1 year ago

For some reason the usual "remove/rescan" method in Linux doesn't work with my card. I can't find a way to reset it over PCIe. Can you suggest a way to reset the TAPA core?

Blaok commented 1 year ago

For some reason the usual "remove/rescan" method in Linux doesn't work with my card. I can't find a way to reset it over PCIe. Can you suggest a way to reset the TAPA core?

Have you ever tried xbutil reset?

vkomenda commented 1 year ago

XRT cannot be installed on my setup because of package conflicts with CUDA. Are the sources available for xbutil?

On Thu, 3 Nov 2022, 06:24 Blaok, @.***> wrote:

For some reason the usual "remove/rescan" method in Linux doesn't work with my card. I can't find a way to reset it over PCIe. Can you suggest a way to reset the TAPA core?

Have you ever tried xbutil reset?

— Reply to this email directly, view it on GitHub https://github.com/UCLA-VAST/tapa/issues/117#issuecomment-1301694163, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKF7IP756OFM6S7BUKAECLWGNLABANCNFSM6AAAAAAQR63ZSY . You are receiving this because you authored the thread.Message ID: @.***>

Blaok commented 1 year ago

XRT cannot be installed on my setup because of package conflicts with CUDA.

You might want to file a bug with Xilinx and ask them to fix the packaging issue. I doubt the conflict is foundational.

Are the sources available for xbutil?

It’s part of the XRT runtime: https://github.com/Xilinx/XRT/blob/e0ba8c664459fa6564817d958d1b55e69a6dc9e5/src/runtime_src/core/tools/xbutil2/xbutil.cpp

vkomenda commented 1 year ago

There is an issue but no-one willing to write a fix.

vkomenda commented 1 year ago

The eot token in my compute kernels with async_mmap does lead to deadlocks in generated designs. I've tried to come up with a custom reset method just for those kernels but weirdly instead of a targeted reset the host machine gets reset instead!

Blaok commented 1 year ago

Have you ever considered cleaning up the eot tokens in TAPA/HLS?

vkomenda commented 1 year ago

Have you ever considered cleaning up the eot tokens in TAPA/HLS?

Would #124 work? It fixes the "destructed with leftovers" warnings. Or would you rather open the stream at the start of processing and not at the end (in which case the warnings will remain)?

Blaok commented 1 year ago

Merged, thanks!