Make feature macro undefs conditional on -cl-ext input
#426
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AGindinson
force-pushed
the
clang-14-feature-macro
branch
from
7466a0a to
9afea85
Compare
|
@hewj03, @haonanya, @JablonskiMateusz, could you please take a look? |
AGindinson
commented
May 15, 2023
AGindinson
force-pushed
the
clang-14-feature-macro
branch
from
9afea85 to
dd96996
Compare
Commits d6563b0, de262c9 and d5a2638 have provided a resolution for `opencl-c-base.h` defining OCL C 3.0 feature macros unconditionally, which interfered with Compute Runtime's platform-dependent definitions of extension lists. An issue with this approach is that `opencl-c-base.h` provides additional defines based on `__opencl_c_atomic_scope_all_devices` being set: ``` memory_scope_all_svm_devices = __OPENCL_MEMORY_SCOPE_ALL_SVM_DEVICES, memory_scope_all_devices = memory_scope_all_svm_devices, ``` (from https://github.com/llvm/llvm-project/blob/release/16.x/clang/lib/Headers/opencl-c-base.h#L387-L392) Simply erasing the macro definition from `opencl-c-base.h` leads to memory scope values' erasure, as the command line definitions aren't available at that moment due to headers' inclusion order. Original commit d6563b0 did mention an alternative approach, which accounts for the above issue: > Once https://reviews.llvm.org/D141297 (LLVM 16) is integrated, an alternative approach could be to pass `-D__undef_<feature_macro>` <...> at the OCL Clang's level (upon detecting the lack of extension-defining inputs to `-cl-ext`). Implement just that in scope of the current commit. Said LLORG patch simply needs to be applied for CClang 14-15 instead of the original "unconditional undef" one. Signed-off-by: Artem Gindinson <[email protected]>
AGindinson
force-pushed
the
clang-14-feature-macro
branch
from
dd96996 to
671e123
Compare
|
would you also add the patch to 15/16 branches? |
Sure, I'll be creating porting CPs soon. For 16, we'd just need the |
AGindinson
pushed a commit
to AGindinson/opencl-clang
that referenced
this pull request
May 16, 2023
Cherry-pick commit 78c5e3f from `ocl-open-140` branch. Signed-off-by: Artem Gindinson <[email protected]>
haonanya
pushed a commit
that referenced
this pull request
May 17, 2023
Cherry-pick commit 78c5e3f from `ocl-open-140` branch. Signed-off-by: Artem Gindinson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commits d6563b0, de262c9 and d5a2638 have provided a resolution for
opencl-c-base.hdefining OCL C 3.0 feature macros unconditionally, which interfered with Compute Runtime's platform-dependent definitions of extension lists.An issue with this approach is that
opencl-c-base.hprovides additional defines based on__opencl_c_atomic_scope_all_devicesbeing set:(from https://github.com/llvm/llvm-project/blob/release/16.x/clang/lib/Headers/opencl-c-base.h#L387-L392)
Simply erasing the macro definition from
opencl-c-base.hleads to memory scope values' erasure, as the command line definitions aren't available at that moment due to headers' inclusion order. Original commit d6563b0 did mention an alternative approach, which accounts for the above issue:Implement just that in scope of the current commit. Said LLORG patch simply needs to be applied for CClang 14-15 instead of the original "unconditional undef" one.