-
Notifications
You must be signed in to change notification settings - Fork 19
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] Fix edge_detection and add it to the lit test-suite #87
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the feeling that now array partitioning is no longer used by the SYCL backend.
Note that it does not really change since it was ignored by HLS before... :-)
@@ -92,10 +101,10 @@ int main(int argc, char* argv[]) { | |||
// to parallel_fors with local sizes | |||
[=]() { | |||
auto gX = xilinx::partition_array<char, 9, | |||
xilinx::partition::complete<0>>({-1, 0, 1, -2, 0, 2, -1, 0, 1}); | |||
xilinx::partition::complete<1>>({-1, 0, 1, -2, 0, 2, -1, 0, 1}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious this change.
Are you changing the API?
Do you think it is better to start at 1?
@@ -258,13 +266,25 @@ def getDeviceCount(device_type): | |||
if xocc == "only": | |||
config.name = 'SYCL-XOCC' | |||
config.test_source_root = config.test_source_root + "/xocc_tests" | |||
run_if_hw="echo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least add some motivating comments
@@ -59,7 +53,7 @@ void dataflow(T functor) { | |||
pipeline way. | |||
*/ | |||
template <typename T> | |||
SYCL_DEVICE_ANNOTATE("xilinx_pipeline") __attribute__((always_inline)) | |||
__SYCL_DEVICE_ANNOTATE("xilinx_pipeline") __attribute__((always_inline)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could even be __TRISYCL
prefix to reduce conflicts with Intel or other SYCL implementations
@@ -348,78 +348,6 @@ struct InSPIRation : public ModulePass { | |||
} | |||
} | |||
|
|||
/// This currently looks through the arguments passed to the SSDM intrinsic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edge_detection was failing because of the first argument was expected of xilinx_partition_array was in the generic address space instead of the default/stack address space.
I also added edge_detection to the test-suite it will run only if opencv4 is found and will only compile in hw_emu because images are too large datasets.