-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SYCL][XilinxFPGA] Known Issues #40
Comments
This also affects SYCL's own structures which are used without an accessor, in particular the partitioned array extension. Example code, error log:
|
CONFLICT (content): Merge conflict in clang/lib/CodeGen/CGLoopInfo.h CONFLICT (content): Merge conflict in clang/lib/CodeGen/CGLoopInfo.cpp
Some bugs on this list have been fixed since it was written.
This is fixed is fixed sw_emu and hw_emu still not in hw.
Both issues are still present issue.
It is now possible to use -g when compiling in SYCL mode. but this will only affect host code.
This issue doesn't occur anymore
This issue doesn't happend anymore on the provided code.
|
I think this is the failing test case your missing: https://github.com/agozillon/sycl/blob/sycl/unified/next-applied-fixes/sycl/test/xocc_tests/issue_related/agdce_ice.cpp hopefully it works now! Great work. |
Thanks, I updated the comment. |
It doesn't seem to cause any issues on real hardware. My Alveo U200 executes the other kernels in the program just fine. |
This is a non-exhaustive list of some larger problems relating to XIlinx FPGA compilation and runtime execution (some with more information than others) that need some thought long term:
Problem: If you create buffers and try to use some SYCL functionality that makes use of some underlying OpenCL functionality to modify the buffers as cl_mem objects before a kernel is invoked (single_task/parallel_for etc.) you'll incur XRT runtime errors e.g.:
[XRT] ERROR: Internal error. cl_mem doesn't map to buffer object
You should be able to see this in action if you comment out the "noop" SYCL kernel in the accessor_copy.cpp test.
Reason: XRT will not consider a device as "active" and use-able until you've loaded a binary as it can't query most of the information it needs, one unfortunate side affect of this is that cl_mem buffers are not appropriately assigned to a device and whenever you try to use something like a handler copy with a sycl accessor/buffer the underlying XRT OpenCL call will not be able to find the buffer in relation to a device (it queries devices for buffers, if they're not found, XRT is not pleased).
Possible fix Ideas:
Work around: Force start the device by using a noop kernel, not ideal and while it works around the issue on hw/sw emu I'm not too sure how real hardware will appreciate this.
Problem: Only 1 queue to a Xilinx FPGA device can exist at once, if you accidentally generate more than one XRT will not be happy.
Problem: All kernels require at least 1 accessor, 0 accessors will cause a compile error in xocc relating to no argument being bound to AXI_GMEM. Not too sure how many use cases there are for no accessors in a kernel but perhaps it shouldn't emit an error like this.
Problem: Cannot compile for Xilinx FPGA with -g, this prevents users using debug mode on the SYCL runtime not just the kernel.
Reason: I believe this is because we're generating a kernel file with a lot of debug code and trying to pipe that through xocc which doesn't really know how to handle all of the debug information.
Possible fix Ideas:
Problem: Related to issue: #32 mixing structures inside of kernels can cause ICE's in one of xocc's passes: aggressive dead code elimination (AGDCE). The relevant minimal triggering SYCL test case for this is issue_related/agdce_ice.cpp.
Reason: Seems to be a problem relating to address space casting from a structure that is implemented outside of a kernel (ergo no address space) and when you try to index an accessor containing several passed in instantiations of the class/struct it will explode the AGDCE pass inside of the compiler as it will try to address space cast.
Status: I am led to believe it's a bug with xocc/Vivado HLS, so it appears to be outside of our jurisdiction, I have forwarded this issue onto someone on their team but it's low priority. May take a while for a fix without some follow up.
Problem: Boost Hana's times::with_index in conjunction with it's overloaded + operator will kill hw_emu and very likely hw as it will not completely optimize and inline with -03 as you would expect (and as it does in a non-SYCL -O3 pass). This leaves some external declarations and calls to functions but no definitions of the functions, so partial optimization/in-lining. Which is a little odd as the definitions exist prior to the -O3 pass and other boost hana functionality is appropriately inlined.
Reason: Current best guess is that the required index argument passed into the lambda passed to and invoked by with_index is the probable cause. It seems like it could be another address space cast related issue. The minimal test case for this issue is the example: boost_hana_functor_arg.cpp inside of the issue_related directory.
So to highlight the issue, In the example there is an variable passed into the lambda from an externally defined function, this then gets used with the + operator. This + operator is overloaded inside of Boost Hana to support compile time usage. This snippet of code should be unrolled and inlined removing all of the Boost Hana related functionality. This doesn't happen and it seems to be because the argument passed into the function and being incremented and used with the value 1 will trigger an address space cast which I think will prevent the appropriate and required optimizations.
This is an an assumption though, based on the fact that the below code works fine:
The index variable and the random constant variable now exist in the same address space and the world all seems to be fine as far as the compiler is concerned.
Some of these "problems" are peculiarities in our FPGA compilation pipeline and non-standard OpenCL implementation and may not necessarily be "problems".
The text was updated successfully, but these errors were encountered: