Skip to content

[SYCL] Disallow zero length arrays in SYCL kernels #1135

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[SYCL] Disallow zero length arrays in SYCL kernels #1135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10429,6 +10429,8 @@ def err_conflicting_sycl_kernel_attributes : Error<
"conflicting attributes applied to a SYCL kernel">;
def err_conflicting_sycl_function_attributes : Error<
"%0 attribute conflicts with '%1' attribute">;
def err_sycl_typecheck_zero_array_size : Error<
"zero-length arrays are not permitted in SYCL kernels">;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use err_typecheck_zero_array_size diagnostics.

Copy link
Contributor Author

@cperkinsintel cperkinsintel Feb 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do, that will output the message "zero-length arrays are not permitted in C++" when a zero length array is encountered from a SYCL kernel ( * ). Is that ok?

I think the "zero-length arrays are not permitted in SYCL kernels" is more accurate. But the decision is yours, @bader

( * ) - Zero length arrays elsewhere will still be accepted ( and I don't think we should change that behavior).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want customize the message, you can make this diagnostics parameterized and emit different wording for SYCL.
I'm fine with the original message.

def err_sycl_x_y_z_arguments_must_be_one : Error<
"%0 X-, Y- and Z- sizes must be 1 when %1 attribute is used with value 0">;
def err_intel_attribute_argument_is_not_in_range: Error<
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Sema/SemaType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,13 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
<< ArraySize->getSourceRange();
ASM = ArrayType::Normal;
}

// SYCL kernels reject zero length arrays
if (getLangOpts().SYCLIsDevice) {
SYCLDiagIfDeviceCode(ArraySize->getBeginLoc(),
diag::err_sycl_typecheck_zero_array_size)
<< ArraySize->getSourceRange();
}
} else if (!T->isDependentType() && !T->isVariablyModifiedType() &&
!T->isIncompleteType() && !T->isUndeducedType()) {
// Is the array too large?
Expand Down
3 changes: 3 additions & 0 deletions clang/test/SemaSYCL/sycl-restrict.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ void usage(myFuncDef functionPtr) {

// expected-error@+1 {{__float128 is not supported on this target}}
__float128 A;

// expected-error@+1 {{zero-length arrays are not permitted in SYCL kernels}}
int BadArray[0];
}

namespace ns {
Expand Down