Skip to content

[SYCL] Throw exception if range/offset of kernel execution exceeds INT_MAX #1713

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

Jump to bottom
Merged
merged 13 commits into from
May 28, 2020
Merged

[SYCL] Throw exception if range/offset of kernel execution exceeds INT_MAX #1713

merged 13 commits into from
May 28, 2020

Conversation

s-kanaev
Copy link
Contributor

@s-kanaev s-kanaev commented May 19, 2020 •
edited
Loading

The exception will only be thrown if __SYCL_ID_QUERIES_FIT_IN_INT macro is defined.

Sergey Kanaev added 4 commits May 19, 2020 18:53
[SYCL] Throw exception if range/offset of kernel execution exceeds IN…
778145b 
…T_MAX

Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
[SYCL] Reword exception message
9833d3d 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Merge branch 'sycl' into private/s-kanaev/id-int-exception
1401a63
[SYCL] Add test
217791d 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
@s-kanaev s-kanaev marked this pull request as ready for review May 21, 2020 08:12
@s-kanaev s-kanaev requested a review from a team as a code owner May 21, 2020 08:12
@s-kanaev s-kanaev requested a review from alexbatashev May 21, 2020 08:12
[SYCL] Fix style issues
298f867 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
alexbatashev
alexbatashev reviewed May 21, 2020
View reviewed changes
Sergey Kanaev added 4 commits May 25, 2020 10:06
[SYCL] Stylistic changes in test
e81218a 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
Merge branch 'sycl' into private/s-kanaev/id-int-exception
c82c6d6
[SYCL] Address comments
26fbe53 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
[SYCL] Address comments
8fbd039 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
keryell
keryell reviewed May 26, 2020
View reviewed changes
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Since this is C++ you should use https://en.cppreference.com/w/cpp/header/limits instead of old INT_MAX C macro

alexbatashev
alexbatashev previously approved these changes May 26, 2020
View reviewed changes
Copy link
Contributor

@alexbatashev alexbatashev left a comment

Choose a reason for hiding this comment

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

LGTM

@bader
Copy link
Contributor

bader commented May 27, 2020

Since this is C++ you should use https://en.cppreference.com/w/cpp/header/limits instead of old INT_MAX C macro

@s-kanaev, are you going to address this?

@s-kanaev
Copy link
Contributor Author

are you going to address this?

I believe not as the contract was about INT_MAX.

bader
bader requested changes May 27, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

are you going to address this?

I believe not as the contract was about INT_MAX.

I don't follow. What "contract" are you talking about?

@s-kanaev
Copy link
Contributor Author

I don't follow. What "contract" are you talking about?

Sorry, a little misunderstanding here.

Since this is C++ you should use https://en.cppreference.com/w/cpp/header/limits instead of old INT_MAX C macro

The comment is addressed.

[SYCL] Address comments.
a8dcc96 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
[SYCL] Fix style issues
cfbf2b6 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
@s-kanaev s-kanaev requested review from alexbatashev and bader May 27, 2020 12:50
[SYCL] Address comments.
c382be7 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>
bader
bader previously approved these changes May 27, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

This looks better. Thanks.

v-klochkov
v-klochkov reviewed May 27, 2020
View reviewed changes
v-klochkov
v-klochkov requested changes May 27, 2020
View reviewed changes
Copy link
Contributor

@v-klochkov v-klochkov left a comment

Choose a reason for hiding this comment

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

Please fix that issue with std::max + windows.h by adding parentheses around max.
This is pretty common issue that, I believe, must have worked-around in SYCL headers.

@s-kanaev @v-klochkov
[SYCL] Resolve issue with 'max' and windows.h
9a9279f 
Signed-off-by: Sergey Kanaev <sergey.kanaev@intel.com>

Co-authored-by: Vyacheslav Klochkov <34946562+v-klochkov@users.noreply.github.com>
@s-kanaev s-kanaev requested review from v-klochkov and bader May 27, 2020 18:41
@bader bader merged commit 08f8656 into intel:sycl May 28, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Jun 2, 2020
Merge remote-tracking branch 'upstream/sycl' into dump_record_layout
fc3b63d 
* upstream/sycl:
  [SYCL] Handle KernelName templated using type with enum template argument (intel#1780)
  [SYCL] Fix KernelNameInfo generated for empty template parameter pack (intel#1775)
  [SYCL] Do not export utility methods from SYCLMemObjT (intel#1768)
  [Driver][SYCL] Fix processing when using -fsycl-link (intel#1765)
  [SYCL][NFC] Remove outdated confusing comment (intel#1779)
  [SYCL][NFC] Wrap classes in .cpp into a namespace to disable external linkage. (intel#1776)
  [SYCL][CUDA] Fixes CUDA unit tests that uses SYCL directly (intel#1763)
  [SYCL][Doc] Fix default device selection rules doc (intel#1769)
  [SYCL][CUDA] Remove pi Event Callback implementation (intel#1735)
  [SYCL] Throw exception if range/offset of kernel execution exceeds INT_MAX (intel#1713)
  [SYCL-PTX] Add intermediate layer to libclc to ease type management (intel#1712)
@s-kanaev s-kanaev deleted the private/s-kanaev/id-int-exception branch September 2, 2020 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keryell keryell keryell left review comments

@alexbatashev alexbatashev alexbatashev approved these changes

@v-klochkov v-klochkov v-klochkov approved these changes

@bader bader Awaiting requested review from bader

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@s-kanaev @bader @alexbatashev @keryell @v-klochkov