[SYCL] Introduce non-variadic version of __spirv_ocl_printf #4779
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
Moved declaration of the built-in from compiler to header files, in order to add a switch (through macro) between two versions of the built-in: variadic and non-variadic one. The problem with variadic version is that it follows C++ argument promotion rules and replaces all floats with doubles, which doesn't look good on HW, which doesn't support doubles. Due to the fact, that we don't know the target HW in advance, we can't selectively disable argument promotion for only some targets and doing so will look like a hack anyway. Therefore, a non-variadic version is introduced, which allows to avoid argument promotion without any further changes to the compiler (and thus, not affecting other variadic functions). Encountering such `printf` call without argument promotion performed should not be a surprise for a backend which consumes SPIR-V, because SPIR-V instruction for `printf` is not variadic, all argument types are statically known and it is legal to have both floats and doubles as arguments at the same time. However, there is no certainty yet in proper support of that instruction in backends: they were mostly designed for OpenCL, where we either have argument promotion or don't have it, but not encountering a mixed case. Because of that, the new version is put under a macro for now, which allows to experiment with it without disrupting existing applications using `printf`.
AlexeySachkov
requested review from
elizabethandrews,
premanandrao,
smanna12 and
a team
as code owners
smanna12
approved these changes
Oct 20, 2021
premanandrao
approved these changes
Oct 20, 2021
|
@intel/llvm-reviewers-runtime, could you please take a look? |
sergey-semenov
approved these changes
Oct 21, 2021
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.
Moved declaration of the built-in from compiler to header files, in
order to add a switch (through macro) between two versions of the
built-in: variadic and non-variadic one.
The problem with variadic version is that it follows C++ argument
promotion rules and replaces all floats with doubles, which doesn't look
good on HW, which doesn't support doubles.
Due to the fact, that we don't know the target HW in advance, we can't
selectively disable argument promotion for only some targets and doing
so will look like a hack anyway.
Therefore, a non-variadic version is introduced, which allows to avoid
argument promotion without any further changes to the compiler (and
thus, not affecting other variadic functions).
Encountering such
printfcall without argument promotion performedshould not be a surprise for a backend which consumes SPIR-V, because
SPIR-V instruction for
printfis not variadic, all argument types arestatically known and it is legal to have both floats and doubles as
arguments at the same time. However, there is no certainty yet in proper
support of that instruction in backends: they were mostly designed for
OpenCL, where we either have argument promotion or don't have it, but
not encountering a mixed case.
Because of that, the new version is put under a macro for now, which
allows to experiment with it without disrupting existing applications
using
printf.