-
Notifications
You must be signed in to change notification settings - Fork 222
Added tests validating clEnqueueSVMMemcpy when source and destination SVM ranges overlap #2593
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
base: main
Are you sure you want to change the base?
Conversation
…ERLAP when source and destination SVM ranges overlap
bashbaug
left a comment
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.
Have you considered testing all three scenarios in one test instead?
This would let us reuse the same context / command queue / etc, even the same SVM allocation if we choose, which means the tests will run faster and the code will be shorter.
| char *data_buf = | ||
| (char *)clSVMAlloc(contextWrapper, CL_MEM_READ_WRITE, dataSize * 2, 0); |
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.
Recommend using the clSVMWrapper here instead, which automatically handles freeing the SVM allocation.
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.
Done
| if (!data_buf) | ||
| { | ||
| log_error( | ||
| "svm_negative_dst_overlap_src: invalid result for clSVMAlloc\n"); | ||
| return TEST_FAIL; | ||
| } |
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.
Consider:
| if (!data_buf) | |
| { | |
| log_error( | |
| "svm_negative_dst_overlap_src: invalid result for clSVMAlloc\n"); | |
| return TEST_FAIL; | |
| } | |
| test_assert_error(data_buf != nullptr, "clSVMAlloc failed"); |
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.
Done
Corrected |
Fixes #2372 according to issue description