Skip to content

[SYCL][FPGA][NFC] Fix num_simd_work_items and reqd_work_group_size argument check #3275

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

Merged
merged 7 commits into from
Mar 7, 2021
Merged

[SYCL][FPGA][NFC] Fix num_simd_work_items and reqd_work_group_size argument check #3275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
436dc40
[SYCL][NFC] Fix num_simd_work_items and reqd_work_group_size argument…
smanna12 Mar 1, 2021
74167d0
Address review comments
smanna12 Mar 1, 2021
044675d
Merge remote-tracking branch 'my_remote/sycl' into FixNumSimdBug
smanna12 Mar 3, 2021
c602a07
Update doc based on review comment
smanna12 Mar 3, 2021
1c35b32
Fix test failure
smanna12 Mar 3, 2021
f6646ca
add comments for each tests
smanna12 Mar 5, 2021
d1555a0
update comment
smanna12 Mar 5, 2021
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
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/AttrDocs.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2421,8 +2421,8 @@ device kernel, the attribute is not ignored and it is propagated to the kernel.
If the`` intel::reqd_work_group_size`` or ``cl::reqd_work_group_size``
attribute is specified on a declaration along with a
intel::num_simd_work_items attribute, the work group size attribute
arguments must all be evenly divisible by the argument specified in
the ``intel::num_simd_work_items`` attribute.
argument (the first argument) must be evenly divisible by the argument specified
Copy link
Contributor

@elizabethandrews elizabethandrews Mar 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For SYCL code isn't this supposed to be the last argument? From the bug report, I thought this attribute is specified as reqd_work_group_size(Z,Y,X) where X is the last argument and must be evenly divisible.

Copy link
Contributor Author

@smanna12 smanna12 Mar 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This will be X - first argument and must be evenly divisible

in the ``intel::num_simd_work_items`` attribute.

.. code-block:: c++

Expand Down
12 changes: 4 additions & 8 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3155,9 +3155,7 @@ static void handleWorkGroupSize(Sema &S, Decl *D, const ParsedAttr &AL) {
int64_t NumSimdWorkItems =
A->getValue()->getIntegerConstantExpr(Ctx)->getSExtValue();

if (!(XDimVal.getZExtValue() % NumSimdWorkItems == 0 ||
YDimVal.getZExtValue() % NumSimdWorkItems == 0 ||
ZDimVal.getZExtValue() % NumSimdWorkItems == 0)) {
if (XDimVal.getZExtValue() % NumSimdWorkItems != 0) {
S.Diag(A->getLocation(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
<< A << AL;
S.Diag(AL.getLoc(), diag::note_conflicting_attribute);
Expand Down Expand Up @@ -3305,14 +3303,12 @@ void Sema::AddSYCLIntelNumSimdWorkItemsAttr(Decl *D,
}

// If the declaration has an [[intel::reqd_work_group_size]] attribute,
// check to see if can be evenly divided by the num_simd_work_items attr.
// check to see if the first argument can be evenly divided by the
// num_simd_work_items attribute.
if (const auto *DeclAttr = D->getAttr<ReqdWorkGroupSizeAttr>()) {
Optional<llvm::APSInt> XDimVal = DeclAttr->getXDimVal(Context);
Optional<llvm::APSInt> YDimVal = DeclAttr->getYDimVal(Context);
Optional<llvm::APSInt> ZDimVal = DeclAttr->getZDimVal(Context);

if (!(*XDimVal % ArgVal == 0 || *YDimVal % ArgVal == 0 ||
*ZDimVal % ArgVal == 0)) {
if (*XDimVal % ArgVal != 0) {
Diag(CI.getLoc(), diag::err_sycl_num_kernel_wrong_reqd_wg_size)
<< CI << DeclAttr;
Diag(DeclAttr->getLocation(), diag::note_conflicting_attribute);
Expand Down
81 changes: 45 additions & 36 deletions clang/test/SemaSYCL/num_simd_work_items_device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,72 +40,77 @@ struct FuncObj {
};

#ifdef TRIGGER_ERROR
// If the declaration has an [[intel::reqd_work_group_size]] or
// [[cl::reqd_work_group_size]] attribute, tests that check if
// the work group size attribute argument (the first argument)
// can be evenly divided by the num_simd_work_items attribute.
struct TRIFuncObjBad1 {
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
[[intel::reqd_work_group_size(5, 3, 3)]] // expected-note{{conflicting attribute is here}}
void
operator()() const {}
};

struct TRIFuncObjBad2 {
[[intel::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
[[intel::reqd_work_group_size(5, 3, 3)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
void
operator()() const {}
};

struct TRIFuncObjBad3 {
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[cl::reqd_work_group_size(5, 5, 5)]] //expected-note{{conflicting attribute is here}}
[[cl::reqd_work_group_size(5, 3, 3)]] // expected-note{{conflicting attribute is here}}
void
operator()() const {}
};

struct TRIFuncObjBad4 {
[[cl::reqd_work_group_size(5, 5, 5)]] // expected-note{{conflicting attribute is here}}
[[cl::reqd_work_group_size(5, 3, 3)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
void
operator()() const {}
};

struct TRIFuncObjBad5 {
[[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
[[intel::reqd_work_group_size(5, 5, 5)]] void
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::reqd_work_group_size(5)]] //expected-note{{conflicting attribute is here}}
void
operator()() const {}
};

struct TRIFuncObjBad6 {
[[intel::reqd_work_group_size(5)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::reqd_work_group_size(5)]] //expected-note{{conflicting attribute is here}}
void
operator()() const {}
};

struct TRIFuncObjBad7 {
[[intel::reqd_work_group_size(5)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::num_simd_work_items(4)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::reqd_work_group_size(3, 64)]] // expected-note{{conflicting attribute is here}}
void
operator()() const {}
};

struct TRIFuncObjBad8 {
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
[[intel::reqd_work_group_size(5, 5)]] // expected-note{{conflicting attribute is here}}
[[intel::reqd_work_group_size(3, 64)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(4)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
void
operator()() const {}
};

// Tests for incorrect argument values for Intel FPGA num_simd_work_items and reqd_work_group_size function attributes
struct TRIFuncObjBad9 {
[[intel::reqd_work_group_size(5, 5)]] // expected-note{{conflicting attribute is here}}
[[intel::num_simd_work_items(3)]] // expected-error{{'num_simd_work_items' attribute must evenly divide the work-group size for the 'reqd_work_group_size' attribute}}
void
operator()() const {}
[[intel::reqd_work_group_size(5, 5, 5)]]
[[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
void operator()() const {}
};

struct TRIFuncObjBad10 {
[[intel::reqd_work_group_size(5, 5, 5)]]
[[intel::num_simd_work_items(0)]] // expected-error{{'num_simd_work_items' attribute requires a positive integral compile time constant expression}}
void operator()() const {}
[[intel::reqd_work_group_size(5, 5, 5)]] void
operator()() const {}
};

struct TRIFuncObjBad11 {
Expand Down Expand Up @@ -155,28 +160,32 @@ struct TRIFuncObjBad18 {
[[intel::reqd_work_group_size(-1)]] // expected-warning{{implicit conversion changes signedness: 'int' to 'unsigned long long'}}
void operator()() const {}
};
#endif // TRIGGER_ERROR

#endif // TRIGGER_ERROR
// If the declaration has an [[intel::reqd_work_group_size]] or
// [[cl::reqd_work_group_size]] attribute, tests that check if
// the work group size attribute argument (the first argument)
// can be evenly divided by the num_simd_work_items attribute.
struct TRIFuncObjGood1 {
[[intel::num_simd_work_items(4)]]
[[intel::reqd_work_group_size(64, 64, 64)]] void
[[intel::reqd_work_group_size(64, 64, 5)]] void
operator()() const {}
};

struct TRIFuncObjGood2 {
[[intel::reqd_work_group_size(64, 64, 64)]]
[[intel::reqd_work_group_size(64, 64, 5)]]
[[intel::num_simd_work_items(4)]] void
operator()() const {}
};

struct TRIFuncObjGood3 {
[[intel::num_simd_work_items(4)]]
[[cl::reqd_work_group_size(64, 64, 64)]] void
[[cl::reqd_work_group_size(64, 64, 5)]] void
operator()() const {}
};

struct TRIFuncObjGood4 {
[[cl::reqd_work_group_size(64, 64, 64)]]
[[cl::reqd_work_group_size(64, 64, 5)]]
[[intel::num_simd_work_items(4)]] void
operator()() const {}
};
Expand All @@ -195,12 +204,12 @@ struct TRIFuncObjGood6 {

struct TRIFuncObjGood7 {
[[intel::num_simd_work_items(4)]]
[[intel::reqd_work_group_size(64, 64)]] void
[[intel::reqd_work_group_size(64, 5)]] void
operator()() const {}
};

struct TRIFuncObjGood8 {
[[intel::reqd_work_group_size(64, 64)]]
[[intel::reqd_work_group_size(64, 5)]]
[[intel::num_simd_work_items(4)]] void
operator()() const {}
};
Expand Down Expand Up @@ -242,8 +251,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 4
Expand All @@ -259,8 +268,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 4
Expand All @@ -276,8 +285,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 4
Expand All @@ -293,8 +302,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK: SYCLIntelNumSimdWorkItemsAttr {{.*}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 4
Expand Down Expand Up @@ -341,8 +350,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 1
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
Expand All @@ -358,8 +367,8 @@ int main() {
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 64
// CHECK-NEXT: IntegerLiteral{{.*}}64{{$}}
// CHECK-NEXT: value: Int 5
// CHECK-NEXT: IntegerLiteral{{.*}}5{{$}}
// CHECK-NEXT: ConstantExpr{{.*}}'int'
// CHECK-NEXT: value: Int 1
// CHECK-NEXT: IntegerLiteral{{.*}}1{{$}}
Expand Down