Skip to content

[SYCL-MLIR] Handle accesses of structure of vector #7767

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 5 commits into from
Dec 15, 2022

Conversation

whitneywhtsang
Copy link
Contributor

@whitneywhtsang whitneywhtsang commented Dec 13, 2022

Since #7706, we decided to not add types under sycl::detail namespace in SYCL dialect.
So sycl::detail::Boolean is represented as llvm structure of vector type.

Pointer of struct is represented as llvm.ptr<struct<...>>, instead of memref<?xstruct<...>>.
Accessing element of the struct uses llvm.getelementptr, which produces llvm.ptr.
If an element of the struct is vector, then pointer to it would be llvm.ptr<vector>.

Pointer of SYCL type is represented as memref<?x!sycl...>.
Accessing element of the SYCL type uses polygeist.subindex, which produces memref.
If an element of the SYCL type is vector, e.g., !sycl.vec, then pointer to it would be memref<vector>.

We need to modify some code that does vector handling, as llvm.ptr<vector> is possible.

@whitneywhtsang whitneywhtsang added the sycl-mlir Pull requests or issues for sycl-mlir branch label Dec 13, 2022
@whitneywhtsang whitneywhtsang self-assigned this Dec 13, 2022
@whitneywhtsang
Copy link
Contributor Author

Depends on #7706. Will rebase after #7706 get merged.

@whitneywhtsang whitneywhtsang changed the title [SYCL-MLIR] Add support of initializing sycl::detail::Boolean [SYCL-MLIR] Add support of constant initializing sycl::detail::Boolean Dec 13, 2022
Signed-off-by: Tsang, Whitney <whitney.tsang@intel.com>
@victor-eds
Copy link
Contributor

I propose replacing this test with a non-SYCL one in the parent directory. Instead of using sycl::detail::Boolean, I'd define a VecWrapper structured type holding an ext vector and store an ext vector of such type in it.

Signed-off-by: Tsang, Whitney <whitney.tsang@intel.com>
Signed-off-by: Tsang, Whitney <whitney.tsang@intel.com>
Signed-off-by: Tsang, Whitney <whitney.tsang@intel.com>
Signed-off-by: Tsang, Whitney <whitney.tsang@intel.com>
@whitneywhtsang
Copy link
Contributor Author

I propose replacing this test with a non-SYCL one in the parent directory. Instead of using sycl::detail::Boolean, I'd define a VecWrapper structured type holding an ext vector and store an ext vector of such type in it.

I created a generic test case, but it is still in sycl, as we need to test when default addrspace is different.

Copy link
Contributor

@victor-eds victor-eds left a comment

Choose a reason for hiding this comment

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

LGTM

@victor-eds
Copy link
Contributor

I'd also change the commit message to make it more generic (not a particular Detail::Boolean issue after all)

@whitneywhtsang whitneywhtsang changed the title [SYCL-MLIR] Add support of constant initializing sycl::detail::Boolean [SYCL-MLIR] Handle accesses of structure of vector Dec 15, 2022
@whitneywhtsang whitneywhtsang merged commit d4e351c into intel:sycl-mlir Dec 15, 2022
@whitneywhtsang whitneywhtsang deleted the boolean branch December 15, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sycl-mlir Pull requests or issues for sycl-mlir branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants