Skip to content

[SYCL] Preserve original message and code of kernel/program build result #1108

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

Merged
merged 10 commits into from
Feb 18, 2020

Conversation

s-kanaev
Copy link
Contributor

Signed-off-by: Sergey Kanaev [email protected]

Copy link
Contributor

@asavonic asavonic left a comment

Choose a reason for hiding this comment

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

Please add a test

Sergey Kanaev added 2 commits February 11, 2020 14:55
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev requested a review from asavonic February 11, 2020 12:00
@s-kanaev s-kanaev force-pushed the private/s-kanaev/fix-msg branch from ee840ee to 4c8e0c7 Compare February 13, 2020 07:55
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev force-pushed the private/s-kanaev/fix-msg branch from 4c8e0c7 to 5495889 Compare February 13, 2020 08:01
Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev force-pushed the private/s-kanaev/fix-msg branch from 4efacf7 to 0c19b88 Compare February 13, 2020 12:11
@s-kanaev s-kanaev changed the title [SYCL] Store original message and code of build result [SYCL] Store original message and code of kernel/program build result Feb 13, 2020
@s-kanaev s-kanaev changed the title [SYCL] Store original message and code of kernel/program build result [SYCL] Preserve original message and code of kernel/program build result Feb 13, 2020
Signed-off-by: Sergey Kanaev <[email protected]>
s-kanaev and others added 2 commits February 13, 2020 16:27
Signed-off-by: Sergey Kanaev <[email protected]>

Co-Authored-By: Mikhail Lychkov <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Signed-off-by: Sergey Kanaev <[email protected]>
Result = e.get_cl_code();
} else {
// Exception constantly adds info on its error code in the message
assert(Msg.find_first_of(e.what()) == 0 && "Exception text differs");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not Msg == e.what()? Can you also check that a message is not empty and Result != 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cl::sycl::exception exception adds appends Code to Msg in form of 0 (CL_SUCCESS). On the second throw it will add the same thing again. I don't do any string operations in this patch.

Signed-off-by: Sergey Kanaev <[email protected]>
@s-kanaev s-kanaev requested a review from asavonic February 17, 2020 14:15
@romanovvlad romanovvlad merged commit 9a34a11 into intel:sycl Feb 18, 2020
/// Equals to true if the Msg and Code are initialized. This flag is added
/// due to the possibility of error code being equal to zero even in case
/// if build is failed and cl::sycl::exception is thrown.
bool FilledIn;
Copy link
Contributor

@bader bader Feb 18, 2020

Choose a reason for hiding this comment

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

Isn't !Msg.empty() == FilledIn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it may be. Though, flag is more explicit data and more understandable to read. On the other hand the struct may have a distinct method bool isFilledIn() const.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for replacing this flag with public member function.
FilledIn seems to duplicate information available in two other fields and maintaining is not necessary.

alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Feb 25, 2020
…ages_docs

* origin/sycl: (1092 commits)
  [CI] Add clang-format checker to pre-commit checks (intel#1163)
  [SYCL][CUDA] Initial CUDA backend support (intel#1091)
  [USM] Align OpenCL USM extension header with the specification (intel#1162)
  [SYCL][NFC] Fix unreferenced variable warning (intel#1158)
  [SYCL] Fix __spirv_GroupBroadcast overloads (intel#1152)
  [SYCL] Add llvm/Demangle link dependency for llvm-no-spir-kernel (intel#1156)
  [SYCL] LowerWGScope pass should not be skipped when -O0 is used
  [SYCL][Doc][USM] Add refactored pointer and device queries to USM spec (intel#1118)
  [SYCL] Update the kernel parameter rule to is-trivially-copy-construc… (intel#1144)
  [SYCL] Move internal headers to source dir (intel#1136)
  [SYCL] Forbid declaration of non-const static variables inside kernels (intel#1141)
  [SYCL][NFC] Remove idle space (intel#1148)
  [SYCL] Improve the error mechanism of llvm-no-spir-kernel (intel#1068)
  [SYCL] Added CTS test config (intel#1063)
  [SYCL] Implement check-sycl-deploy target (intel#1142)
  [SYCL] Preserve original message and code of kernel/program build result (intel#1108)
  [SYCL] Fix LIT after LLVM change in community
  Translate LLVM's cmpxchg instruction to SPIR-V
  Add volatile qualifier for atom_ builtins
  Fix -Wunused-variable warnings
  ...
@s-kanaev s-kanaev deleted the private/s-kanaev/fix-msg branch March 12, 2020 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants