Add a useful warning to warn for any functionality which isn't supported without generic address space #2728
Labels
Comments
|
Hi @romanovvlad , this issue was created as a follow up for #2722 to add a new warning based on @rolandschulz's input. |
|
Hi @srividya-sundaram, sorry, not sure how it happened. Reopened. |
|
It seems it happened because I've merged the PR this issue is linked to. https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue |
|
This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days. |
|
This issue was closed because it has been stalled for 30 days with no activity. Please, re-open if the issue still exists. |
jsji
pushed a commit
that referenced
this issue
Oct 10, 2024
This storage class is used for function pointers. It's added as based on cl_intel_function_pointers specification, it is not guaranteed that sizeof(void(*)(void) == sizeof(void *) - to allow consumers use this fact, we cannot say that function pointer belongs to the same storage class as data pointers. It wasn't added during initial implementation, now it's time to fill this gap. As it would be a breaking change its generation is added only under -spirv-emit-function-ptr-addr-space option. Also SPIR-V consumer may pass this option during reverse translation to get new address space even in a case, when OpConstantFunctionPointerINTEL doesn't reside in CodeSectionINTEL storage class. Expected behavior: No option is passed to the forward translation stage and function pointers are in addrspace(9): no CodeSectionINTEL storage class in SPIR-V The option is passed to the forward translation stage and function pointers are in addrepace(9): CodeSectionINTEL storage class is generated No option is passed to the reverse translation stage: function pointers are in private address space The option is passed to the reverse translation stage: function pointers are in addrspace(9) Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_function_pointers.asciidoc The previous approach: #1392 Original commit: KhronosGroup/SPIRV-LLVM-Translator@46c9eb9ea9b4c6f
jsji
pushed a commit
that referenced
this issue
Oct 10, 2024
This storage class is used for function pointers. It's added as based on cl_intel_function_pointers specification, it is not guaranteed that sizeof(void(*)(void) == sizeof(void *) - to allow consumers use this fact, we cannot say that function pointer belongs to the same storage class as data pointers. It wasn't added during initial implementation, now it's time to fill this gap. As it would be a breaking change its generation is added only under -spirv-emit-function-ptr-addr-space option. Also SPIR-V consumer may pass this option during reverse translation to get new address space even in a case, when OpConstantFunctionPointerINTEL doesn't reside in CodeSectionINTEL storage class. Expected behavior: No option is passed to the forward translation stage and function pointers are in addrspace(9): no CodeSectionINTEL storage class in SPIR-V The option is passed to the forward translation stage and function pointers are in addrepace(9): CodeSectionINTEL storage class is generated No option is passed to the reverse translation stage: function pointers are in private address space The option is passed to the reverse translation stage: function pointers are in addrspace(9) Spec: https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_function_pointers.asciidoc The previous approach: #1392 Original commit: KhronosGroup/SPIRV-LLVM-Translator@46c9eb9ea9b4c6f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Input from @rolandschulz
Raw pointers for SYCL_EXTERNAL are prohibited without generic address space. Generic address space doesn't exist in 1.2.1 and is optional in 2020. Our implementation isn't spec compliant to 1.2.1 because we use generic address space (among other).
It seems useless to warn about a detail (SYCL_EXTERNAL with pointers) if the base functionality (pointers using inference rules vs generic address space) isn't implemented spec compliant and the reason for the spec rule is based on the assumption of the implementation.
And this diagnostic has no practical value to users. We aren't warning about something which has any impact for our implementation based on generic address space. And it isn't useful for portability to other implementation. Because this is only a portability concern if the other implementation doesn't support generic address space.
But in that case a useful warning needs to warn for any functionality which isn't supported without generic address space (using the SYCL inference rules instead). This is among other SYCL_EXTERNAL with raw pointer, assignment of raw pointers which can't be inferred (e.g. in conditionals), function ptrs with raw pointers. That warning would allow a user to target implementation which don't support generic address space. And as part of that warning its useful to warn for SYCL_EXTERNAL. But just warning for SYCL_EXTERNAL without also warning for all other features which depend on generic address space is useless.
We might want to create a new issue to add such a useful warning and partially revert this commit and make it a part of such a new optional warning. I'm not sure whether users need that or would just use a different SYCL implementation without generic address space if they need to get diagnostic regarding features not available without generic address space.
The text was updated successfully, but these errors were encountered: