-
Notifications
You must be signed in to change notification settings - Fork 747
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
[SYCL][USM] Initial Support for USM #256
Conversation
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 pull requests contains 8 commits and most of them are merge commits.
Please, make a single commit in PR, rebase you branch on the tip of the sycl
branch and resolve the conflict.
@@ -1,4 +1,4 @@ | |||
// RUN: %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl | |||
// RUN: env ENABLE_INFER_AS=1 %clang -std=c++11 -fsycl %s -o %t.out -lstdc++ -lOpenCL -lsycl |
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.
AFAIK, this variable is not needed.
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.
It is. Adding pointers requires this mode to be enabled in the compiler. Otherwise conversations between global/generic aren't properly handled.
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.
Also, to pass Please, add [SYCL] and [USM] tags to your commit title to make it easier to find in the history. |
1) Support pointer types in Integration Header 2) Put kernel pointer params in the global addr space 3) Fix CheckSYCLType to support proper recursion through pointers Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Modify test to use generic address space switch in order to pass. Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
The idea looks good. |
// CHECK-OLD: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32*{{.*}}){{.*}} !dbg [[KERNEL:![0-9]+]] {{.*}}{ | ||
// CHECK-NEW: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32 addrspace(4)*{{.*}}){{.*}} !dbg [[KERNEL:![0-9]+]] {{.*}}{ | ||
|
||
// CHECK: define{{.*}} spir_kernel {{.*}}void @_ZTSZ4mainE15kernel_function(i32 addrspace(1)*{{.*}}){{.*}} !dbg [[KERNEL:![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.
Interesting.
@asavonic, this looks like a bug with address space handling.
- Note, clang emit OpenCL kernel with "generic pointer" argument.
- Clang does not emit warnings about captured pointers.
I think both these issues should be fixed for the cases when USM feature is not available.
…shifted Add a comment when there is a shifted value, add x9, x0, #291, lsl #12 ; =1191936 but not when the immediate value is unshifted, subs x9, x0, #256 ; =256 when the comment adds nothing additional to the reader. Differential Revision: https://reviews.llvm.org/D107196
…n a loop When untagging the stack, the compiler may emit a sequence like: ``` .LBB0_1: st2g sp, [sp], #32 sub x8, x8, #32 cbnz x8, .LBB0_1 stg sp, [sp], #16 ``` These stack adjustments cannot be described by CFI instructions. This patch disables merging of SP update with untagging, i.e. makes the compiler use an additional scratch register (there should be plenty available at this point as we are in the epilogue) and generate: ``` mov x9, sp mov x8, #256 stg x9, [x9], #16 .LBB0_1: sub x8, x8, #32 st2g x9, [x9], #32 cbnz x8, .LBB0_1 add sp, sp, #272 ``` Merging is disabled only when we need to generate asynchronous unwind tables. Reviewed By: eugenis Differential Revision: https://reviews.llvm.org/D114548
Modifies clang to supports pointers.
Adds pointers to the Integration header object types in clang/sycl rt.
Fixes a couple of tests to work properly with these changes.