-
Notifications
You must be signed in to change notification settings - Fork 769
[SYCL] Fix static|dynamic_address_cast
to generic
#15394
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
Conversation
SPIRV operations are defined such that `OpGenericCastToPtr` and `OpGenericCastToPtrExplicit` cannot be used when target `Storage Class` is `Generic`, yet we were generating such code. This PR fixes that.
static|dynamic_address_cast
to generic
Fix castsstatic|dynamic_address_cast
to generic
// CHECK-LABEL: define dso_local spir_func void @_ZN14static_as_cast20to_generic_decoratedEPi( | ||
// CHECK-SAME: ptr addrspace(4) dead_on_unwind noalias nocapture writable writeonly sret(%"class.sycl::_V1::multi_ptr.1") align 8 [[AGG_RESULT:%.*]], ptr addrspace(4) noundef [[P:%.*]]) local_unnamed_addr #[[ATTR2:[0-9]+]] !srcloc [[META22:![0-9]+]] !sycl_fixed_targets [[META7]] { | ||
// CHECK-NEXT: [[ENTRY:.*:]] | ||
// CHECK-NEXT: store ptr addrspace(4) [[P]], ptr addrspace(4) [[AGG_RESULT]], align 8, !tbaa [[TBAA23:![0-9]+]], !alias.scope [[META25:![0-9]+]] |
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.
Use per-commit diff to see how this PR affects code generation here. First commit is test-only without functional changes.
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.
LGTM, but I do have a question.
// TODO: Remove this restriction. | ||
static_assert(std::is_same_v<ElementType, remove_decoration_t<ElementType>>, | ||
"The extension expect undecorated raw pointers only!"); |
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.
Out of curiosity: is this something that would be difficult to fix? Why not just use remove_decoration_t
to create an undecorated raw pointer here, and then use the undecorated pointer for the rest of the function?
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 plan to do that in the next PR(s). It has to come with the doc change as well.
This became possible sometime after one/all of * intel#15394 * intel#15543 * intel#16604
SPIRV operations are defined such that
OpGenericCastToPtr
andOpGenericCastToPtrExplicit
cannot be used when targetStorage Class
is
Generic
, yet we were generating such code. This PR fixes that.