-
Notifications
You must be signed in to change notification settings - Fork 769
[NFC][SYCL] Simplify the error checking of arrays by only visiting 1x. #2344
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
Conversation
Just a cleanup task I saw that should happen. None of this is really necessary to be public, so make them all private.
The current visitor requires that we go through our diagnostics checkers once per element. This is expensive, so this patch makes it an opt-out as to whether we should visit each element, or only the 1st.
Please forgive there being two commits, the first is covered in this review: #2345 |
I've now merged this with the dependent patch, so the 'diff' is now correct. |
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.
LGTM, thanks
for (int64_t Count = 0; Count < ElemCount; Count++) { | ||
VisitElement(nullptr, FD, ET, handlers...); | ||
|
||
assert(ElemCount > 0 && "SYCL prohibits 0 sized arrays"); |
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.
So, you crash the compiler when a user uses a array of size 0?
And what when not compiled with assertions?
What if this happens by meta-programming on a kernel which is not executed anyway?
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.
We prohibit size-zero arrays. If the user attempts to use a size-zero array in a kernel, they get a diagnostic.
…ported [CTS] Skip images and fill2D tests if feature is not supported
[CTS] Skip images and fill2D tests if feature is not supported
The current visitor requires that we go through our diagnostics checkers
once per element. This is expensive, so this patch makes it an opt-out
as to whether we should visit each element, or only the 1st.