-
Notifications
You must be signed in to change notification settings - Fork 734
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
[NFCI][SYCL] Support multi_ptr in convertToOpenCLType #12693
[NFCI][SYCL] Support multi_ptr in convertToOpenCLType #12693
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
90374db
to
48a6de6
Compare
using DecoratedPtrTy = decltype(DecoratedPtr); | ||
using ElemTy = remove_decoration_t<std::remove_pointer_t<DecoratedPtrTy>>; | ||
|
||
using TargetElemTy = SelectBlockT<ElemTy>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If ElemTy has cv qualifier, it looks like SelectBlockT is going to drop that qualifier.
Maybe my understanding is wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, but I don't think it's a regression caused by this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
NIT: In that case it will be nice to have some todo maybe, otherwise I guess it will be forgotten.
using DecoratedPtrTy = decltype(DecoratedPtr); | ||
using ElemTy = remove_decoration_t<std::remove_pointer_t<DecoratedPtrTy>>; | ||
|
||
using TargetElemTy = SelectBlockT<ElemTy>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
NIT: In that case it will be nice to have some todo maybe, otherwise I guess it will be forgotten.
Pre-commit CI failure on Win:
Github search (https://github.com/search?q=repo%3Aintel%2Fllvm+Graph%2FRecordReplay%2Fbarrier_with_work.cpp&type=pullrequests) didn't find anything but it doesn't look like it's related to this PR either. Tagging @intel/sycl-graphs-reviewers for awareness. I believe it's very unlikely this has been caused by this PR so I'm going to merge it and monitor post-commit afterwards. If it fails, I'll revert. I don't think it would be beneficial to just restart pre-commit and increase the load on CI. |
No description provided.