- User Since
- Sep 23 2020, 3:53 AM (37 w, 3 d)
Wed, Jun 9
@svenvh, are you going to add header-like output emitting?
Tue, Jun 8
Fri, Jun 4
Tue, Jun 1
Mon, May 31
Thu, May 27
Wed, May 26
Fri, May 21
Thu, May 20
Wed, May 19
Drop one more formatting change from Sema.cpp
Drop formatting changes from Sema.cpp
Mon, May 17
Rebase after https://reviews.llvm.org/D100976; addressed review comments and updated docs.
Fri, May 14
May 7 2021
May 6 2021
Rebased to use ::validateOpenCLTarget. Decided to use explicit simulatneous extensions/feature disabling in command line due to API spec limitations:
May 4 2021
Thanks! Looks good to me in general. See a comment.
Apr 28 2021
Not sure what do we want to achieve with this? Do you want to point out that the code might be somehow less portable let's say between clang revisions, etc?
Add test for C++ for OpenCL diagnostics
Apr 27 2021
Generally looks good to me, but maybe a test needed, see a comment. Thanks! But I'm still not sure about completely removing pragmas for type declarations (see https://reviews.llvm.org/D100980#2719196).
When the pragma is parsed we can't know why it is in the code to be able to issue any warning.
I am not sure, to be honest I personally think the extension pragma is a spec failure as it is not specified properly or to allow reasonable implementation
Apr 23 2021
We could of course keep it just for this particular case of doubles, but even half is allowed in certain circumstances without the pragma and it is still an extension. https://godbolt.org/z/K34sP81nx
Use LangOptions::getOpenCLVersionTuple() to provide diagnostics of OpenCL version
For cl_khr_fp64 I still want to keep the pragma for the other use case - to convert double literal into single-precision (https://github.com/KhronosGroup/OpenCL-Docs/issues/578). The reason why I think it could be useful is that the precision change might lead to a different result depending on the precision of the calculation. So I think pragmas could be useful to control whether double literal is single-precision or not to avoid this problem occur when code is compiled for different targets?
Addressed comments, did some more refactoring. Is it OK to have "2.0" diagnostics for C++ for OpenCL?
do you think it is valuable to keep this behavior at all?
Apr 22 2021
Please see https://reviews.llvm.org/D101087.
Btw I am not suggesting removing the pragma. We will still have to parse it for backward compatibility. I am only dropping the requirement of using it in order to call get_kernel_max_sub_group_size_for_ndrange or get_kernel_sub_group_count_for_ndrange when the extension is supported. Because I don't see why this would be needed especially that all other functions from subgroups can be called without the pragma https://godbolt.org/z/MYavchPT3.
Same as for https://reviews.llvm.org/D100984, cl_khr_fp64 wasn't always core and thus it requires pragma for OpenCL C < 1.2 versions.
I wish it could be true... But cl_khr_subgroups still requires pragma in some versions as it wasn't a core feature earlier (https://www.khronos.org/registry/OpenCL/sdk/2.2/docs/man/html/cl_khr_subgroups.html).
Apr 21 2021
Apr 15 2021
Great! And thanks for fixing misprint :)
My main idea was to provide an interface which will not make users to specify -cl-ext=+__opencl_c_fp64,+cl_khr_fp64/ -cl-ext=-__opencl_c_fp64,-cl_khr_fp64 if they need to enable/disable functionality in OpenCL C 3.0 because I believe that is not a right thing to do: why anyone should care about those extensions if there are features already? Also, this may lead to confusions since, for example, __opencl_c_subgroups and cl_khr_subgroups are not the same (subgroup independent forward progress is required in extension while it is optional in OpenCL C 3.0, thus implementation may support the extension but not the feature).
Apr 9 2021
Remove unrelated change, fixed error in test to check only cl_khr_fp64 for OpenCL C < 1.2
Addressed comments, rebased after refactoring: https://reviews.llvm.org/D97058
Apr 7 2021
Thanks for working on this! Looks good to me in general. I have a small comment.
Apr 1 2021
Thanks for feedback!
Mar 30 2021
Looks good to me. Thanks!
Thanks for the patch! Sorry for the delay
Mar 16 2021
I have one more though.
Mar 15 2021
@Anastasia, thanks for landing this! Sorry for delay, I though that this can wait until Monday
Do you also get fatal error: 'opencl-c-base.h' file not found? If so, we might need at least to file a clang bug that we should look at before the next release.
Mar 12 2021
Yes, I was able to reproduce the failure with -target arm64-apple-macosx flag, I provided the fix to set explicit target: https://reviews.llvm.org/D98539.
Yeah, I'm looking into it, but I can't reproduce it locally: the test passes at x86_64 linux system. I'll revert the change if it takes too much time to investigate what's going on. Thanks.
Mar 10 2021
Ok, addressing in a separate patch is reasonable, but why do you think that we will break backward compatibility? My current worry is that the implementation is so messy and inconsistent that it will take us longer time if we do the incremental steps. Also, we risk introducing the regressions since it is so hard to make sense out of it.
Replaced atomic_double implicit definition
Corrected some mistakes, added a test for diagnosing undeclared identifiers when a extension is unsupported. Generally leaving the change as it is as completely removing pragma may break backward compatibility now: let's do it in a separate patch and notify everyone before doing that. Also, we should wait before spec will be finalized.
Mar 4 2021
Mar 3 2021
Check 'isEnabled' is now private: it is used only for non-core or non-optional core features;
creation of implicit type definitions is guarder with extension support check; minor refactoring
Mar 2 2021
Thanks for working on this. LGTM in general. I'll let Marco comment on the latest changes.
Feb 25 2021
I see the update regaring this functionality on github issue: https://github.com/KhronosGroup/OpenCL-Docs/issues/82#issuecomment-785496025. Is it a right way to reflect this information in`OpenCLExtensions.def`?
FYI, I would even be ok if we remove the need for enabling non-core
Feb 20 2021
Feb 19 2021
Thanks! I review is shortly. As for now, I also was doing some refactoring: https://reviews.llvm.org/D97058. Check-clang passes, as I see there are no conflicts for now.
Feb 17 2021
Feb 16 2021
Thanks for review. Last change is a small fix for if clang-tidy warnings
Fix clang-tidy warnings
Feb 15 2021
Feb 12 2021
Rewrote 'RemoveAddressSpaceFromPtr' to return canonical type
Feb 11 2021
Feb 10 2021
Thanks for review. Just to be clear: can I proceed with this? Or we should discuss it first? As I see possible this change can be abandoned after discussion.