Skip to content

[SYCL] Change lowering of 'cl::sycl::select' into SPIR-V #904

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

Conversation

AlexeySachkov
Copy link
Contributor

Previously, OpSelect from SPIR-V core spec was used, but comparing to
the SYCL definition of 'select' built-in, it behaves
differently and even takes different arguments.

select instruction from OpenCL Extended Instruction Set must be used
instead.

Description of OpSelect from SPIR-V core spec:

Condition must be a scalar or vector of Boolean type.
If Condition is a scalar and true, the result is Object 1. If
Condition is a scalar and false, the result is Object 2.

If Condition is a vector, Result Type must be a vector with the same
number of components as Condition and the result is a mix of Object 1
and Object 2: When a component of Condition is true, the corresponding
component in the result is taken from Object 1, otherwise it is taken
from Object 2.

Description of select ExtInst:

For each component of a vector type, the result is a if the most
significant bit of c is zero, otherwise it is b.

For a scalar type, the result is a if c is zero, otherwise it is b.

c must be integer or vector(2,3,4,8,16) of integer values.

The latter perfectly matches both SYCL and OpenCL specs.

Note: previous implementation emulate ExtInst select behavior over
OpSelect by evaluating MSB of each vector compontent in C++ code, so, it
is functionally correct. However, it uses ext-vectors of booleans, which are
unsupported by OpenCL and confuse underlying OpenCL compilers sometimes
(crashes and hangs experienced in complex applications because of this).

alexbatashev
alexbatashev previously approved these changes Dec 5, 2019
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, my comment can be ignored

Previously, `OpSelect` from SPIR-V core spec was used, but comparing to
the SYCL definition of 'select' built-in, it behaves
differently and even takes different arguments.

`select` instruction from OpenCL Extended Instruction Set must be used
instead.

Description of `OpSelect` from SPIR-V core spec:

> Condition must be a scalar or vector of Boolean type.
> If Condition is a scalar and true, the result is Object 1. If
> Condition is a scalar and false, the result is Object 2.
>
> If Condition is a vector, Result Type must be a vector with the same
> number of components as Condition and the result is a mix of Object 1
> and Object 2: When a component of Condition is true, the corresponding
> component in the result is taken from Object 1, otherwise it is taken
> from Object 2.

Description of `select` ExtInst:

> For each component of a vector type, the result is a if the most
> significant bit of c is zero, otherwise it is b.
>
> For a scalar type, the result is a if c is zero, otherwise it is b.
>
> c must be integer or vector(2,3,4,8,16) of integer values.

The latter perfectly matches both SYCL and OpenCL specs.

Note: previous implementation emulate ExtInst select behavior over
OpSelect by evaluating MSB of each vector compontent in C++ code, so, it
is functionally correct. However, it uses ext-vectors of booleans, which are
unsupported by OpenCL and confuse underlying OpenCL compilers sometimes
(crashes and hangs experienced in complex applications because of this).

Signed-off-by: Alexey Sachkov <[email protected]>
@bader bader merged commit 23cbe7f into intel:sycl Dec 6, 2019
@AlexeySachkov AlexeySachkov deleted the private/asachkov/fix-select-spirv-lowering branch December 6, 2019 13:33
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request May 18, 2021
)

Expose SPIRVToOCL12, SPIRVToOCL20 interfaces in SPIRVToOCL.h

This "recommit"s intel#904, which was accidentally "revert"ed by intel#840

Signed-off-by: Yilong Guo <[email protected]>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@6171e51
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.

3 participants