-
Notifications
You must be signed in to change notification settings - Fork 130
Add a test for SYCL_INTEL_local_memory extension #176
Conversation
Depends on and checks the changes from intel/llvm#3329 and intel/llvm#3228 |
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.
This looks great.
@jbrodman suggested that it might be a good idea to demonstrate some alternative ways to use the pointer, in case developers are basing their code on our tests, e.g.: auto Ptr = group_local_memory_for_overwrite<int[WgSize]>(Item.get_group()); and Foo& Ref = *group_local_memory(Item.get_group(), ...); If all of these syntax options are functionally equivalent (as I think they are) then we might not need to actually test them, but it might still be worth adding a comment or two to let readers know other options are available. |
@Pennycook Added a comment on this. |
@@ -0,0 +1,95 @@ | |||
// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out |
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.
New directory is added. Could you please set correct code owner (in .github/CODEOWNERS)
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.
Added myself as the code owner. @Pennycook Do you mind being set as the second code owner for the group local memory tests? (I this can be done as a separate PR, so that it does not block merging this one).
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.
@Pennycook Do you mind being set as the second code owner for the group local memory tests?
Fine by me.
No description provided.