[SYCL] Move deferred emit check to after "Can be emitted" checks. #2102
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous implementation caused function declarations to be attempted to be emitted if they were required to be emitted (such as in the case of -femit-all-decls), which caused a crash. This patch moves the check until after the 'can be emitted' checks for function/var decls, and limits it to only functions that are sycl-devices (or, in the case of implicit attributes, called from a sycl-device function).
erichkeane
requested review from
elizabethandrews and
premanandrao
as code owners
premanandrao
previously approved these changes
Jul 13, 2020
premanandrao
previously approved these changes
Jul 13, 2020
|
I don't really understand the test failures, but I've relaxed the check a little to be over-kill like it was before. Hopefully this will fix the SYCL test. My look at the SYCL test failure showed that the symbol was only emitted from the CFE 1x, so its not clear why the linking process thinks there are multiples. |
jsji
pushed a commit
that referenced
this pull request
Aug 11, 2023
It's duplicating existing information in the SPIR-V and it has complex structure. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@d498f48
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation caused function declarations to be attempted
to be emitted if they were required to be emitted (such as in the case
of -femit-all-decls), which caused a crash.
This patch moves the check until after the 'can be emitted' checks for
function/var decls, and limits it to only functions that are
sycl-devices (or, in the case of implicit attributes, called from a
sycl-device function).