Skip to content

[SYCL][ESIMD] Fix GenXIntrinsic lowering for vectors used as scalars #9211

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 2 commits into from
Apr 27, 2023

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Apr 25, 2023

In most cases, we see the following IR for vector spirv intrinsics:

%0 = load <3 x i64>, <3 x i64> addrspace(4)* @__spirv_BuiltInGlobalInvocationId, align 32
%1 = extractelement <3 x i64> %0, i64 0
// some use of %1, ex:
%2 = icmp ult i64 %0, 50

However, if only the first element is used, this may be optimized to:

%0 = load i64, addrspace(4)* @__spirv_BuiltInGlobalInvocationId, align 32
// some use of %0, ex:
%1 = icmp ult i64 %0, 50

In the latter case, if we try to insert a cast due to different types between the real intrinsic type and the global variable type, make sure to use the load type, because the use type is arbitrary code and may not be the same as the global variable type.

This is fine for the case with ExtractElement because in that case the type of the EE will be based on the global variable type.

In most cases, we see the following IR for vector spirv intrinsics:
```c++
%0 = load <3 x i64>, <3 x i64> addrspace(4)* @__spirv_BuiltInGlobalInvocationId, align 32
%1 = extractelement <3 x i64> %0, i64 0
// some use of %1, ex:
%2 = icmp ult i64 %0, 50
```

However, if only the first element is used, this may be optimized to:
```c++
%0 = load i64, addrspace(4)* @__spirv_BuiltInGlobalInvocationId, align 32
// some use of %1, ex:
%1 = icmp ult i64 %0, 0
```

In the latter case, if we try to insert a cast due to different types between the real intrinsic type and the global variable type,
make sure to use the load type, because the use type is arbitrary code and may not be the same as the global variable type.

This is fine for the case with ExtractElement because in that case the return will be based on the global variable type.

Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex temporarily deployed to aws April 25, 2023 23:56 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to aws April 26, 2023 03:47 — with GitHub Actions Inactive
@sarnex
Copy link
Contributor Author

sarnex commented Apr 26, 2023

Failed Tests (1):
  SYCL :: Plugin/level_zero_imm_cmdlist_per_thread.cpp

********************
Failed Tests (1):
  SYCL :: Regression/same_unnamed_kernels.cpp

not related

@sarnex sarnex marked this pull request as ready for review April 26, 2023 13:34
@sarnex sarnex requested a review from a team as a code owner April 26, 2023 13:34
Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex temporarily deployed to aws April 26, 2023 18:19 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to aws April 26, 2023 22:41 — with GitHub Actions Inactive
@v-klochkov
Copy link
Contributor

This fail is unrelated to this PR with changes in ESIMD lowering.
SYCL :: Regression/same_unnamed_kernels.cpp

@v-klochkov v-klochkov merged commit 637d3d5 into intel:sycl Apr 27, 2023
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.

2 participants