-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL] Deny support of SPV_INTEL_usm_storage_classes for all targets … #1986
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
NOTE: #1985 - need to be merged first |
…but FPGA Disable SPV_INTEL_usm_storage_classes by default since it adds new storage classes that represent global_device and global_host address spaces, which are not supported for all targets. With the extension disabled the storage classes will be lowered to CrossWorkgroup storage class that is mapped to just global address space. The extension is enabled only for FPGA with a flag 'enable-usm-address-spaces' passed to the driver. Signed-off-by: Dmitry Sidorov <[email protected]>
Signed-off-by: Dmitry Sidorov <[email protected]>
ba2ef69
to
dc84a4c
Compare
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.
The patch mostly LGTM, what's probably lacking is a positive test of -fenable-usm-address-spaces
.
I was thinking I had it, thanks for pointing out! |
Signed-off-by: Dmitry Sidorov <[email protected]>
@AGindinson I have added a positive test case in sycl-offload.c. Do you think it's necessary to add it in sycl-offload-intelfpga.cpp as well? I think the last test is more about aocx generation than about options. |
I agree, |
Signed-off-by: Dmitry Sidorov <[email protected]>
Signed-off-by: Dmitry Sidorov <[email protected]>
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
Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
) Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
Currently, we always convert SPIR-V bultins to globals for forward translation and to functions for reverse translation. I have a use case where I want to keep them as globals for reverse translation, so I added this mode. Implementations for both cases already existed, I just consolidated them and added the option. Signed-off-by: Sarnie, Nick <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@730eaf0
…but FPGA
Disable SPV_INTEL_usm_storage_classes by default since it adds new
storage classes that represent global_device and global_host address
spaces, which are not supported for all targets. With the extension
disabled the storage classes will be lowered to CrossWorkgroup storage
class that is mapped to just global address space.
The extension is enabled only for FPGA with a flag 'enable-usm-address-spaces'
passed to the driver.
Signed-off-by: Dmitry Sidorov [email protected]