- User Since
- Apr 12 2019, 2:29 AM (180 w, 5 d)
Jan 13 2022
Thanks! Now committed.
Jan 10 2022
Thanks for the review. I've now addressed the comments, PTAL.
Jan 7 2022
I've done my prototyping assuming support for 64-bit integers but, unfortunately, some targets of interest don't support 64-bit yet so I will likely end up implementing support for both paths (64-bit integers and uvec2) in clspv. I've mapped the clspv64 target to the version that doesn't require 64-bit integers for compatibility, to avoid carrying two builds of the library in clspv (each build adds about 2MB to the binary size). Looking at both implementations of fma, I'm not overly concerned about always using the version that doesn't require support for 64-bit integers for now. Both will be excruciatingly slow compared to hardware support anyway. Longer-term we can either assume support for 64-bit integers once more prevalent in the Vulkan ecosystem or introduce an additional variant of the library but I'd rather treat this as a future optimisation if that works for you.
Jan 5 2022
Adding missing clspv64 symlink to the diff.
Nov 4 2021
Aug 19 2021
Mar 30 2021
Thanks! I'll commit the change.
@azabaznov Without this change, opencl-c.h cannot be parsed with -cl-std=CL3.0 as the write_only image3d_t type is not enabled. The type is currently enabled via cl_khr_3d_image_writes. See https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/OpenCLImageTypes.def#L68. It may be that we want to redesign this such that the type be enabled by the feature macro (or via another mechanism) and have the extension enable the feature macro internally but this would require more thinking and is probably best done as a follow-up IMHO (maybe as part of https://reviews.llvm.org/D92004 or a pre-requisite thereof?).
Mar 28 2021
Thanks! I'll wait for a couple of days before committing then.
Mar 26 2021
Oct 14 2020
Committed as acb7827d6217000541c5c0ce2b03049e4f49d23f
Nov 22 2019
Thanks for the feedback.
As discussed offline, let's only enable restrict under a compatibility flag. PTAL.
Nov 14 2019
Committed as e73177ea5fd611026abcbaecc6232eee8d8d2ed8.
Nov 13 2019
Oct 4 2019
Something I should probably have explained in the commit message is that the functionality provided by restrict is already present in C++, where it is already exposed using different keywords. If you look at clang/include/clang/Basic/TokenKinds.def, there are __restrict__ and __restrict aliases that end up producing tok::kw_restrict. The only thing this patch does is enabling the restrict keyword in C++ for OpenCL for compatibility with OpenCL C (see more on that on the bug). I don't expect any special handling will be required since the functionality is already supported both in C++ and OpenCL C.
Oct 3 2019
There doesn't seem to be a test for this (enabling restrict in C++ doesn't make any existing test fail). I couldn't find any test checking that OpenCL or C++/OpenCL keywords generally aren't enabled in C or C++. Doing this generically would essentially equate to writing unit tests for getKeywordStatus.
Sep 11 2019
Jul 24 2019
Jul 23 2019
Hmm, maybe we need to make sure that one of the tests is using a C++ feature and building with CLC++. This would have caught the mistake.
Thanks for doing this!
Jul 17 2019
Would it be ok if I fix those in a separate commit? I would really like to commit the core part before the release branch is taken.
Very useful to have all of this documented! Thanks!
Jun 18 2019
Friendly ping :).
May 31 2019
May 24 2019
Committed as r361641.
Yes, indeed :).
May 15 2019
May 9 2019
The comments could use a bit more polishing but nothing that justifies making another iteration IMHO. LGTM!
May 8 2019
May 7 2019
May 3 2019
May 2 2019
Clean up the tests.
Most of these are pre-existing issues with the tests but I agree they're worth fixing. I'll update the tests.
May 1 2019
Replaced the AST dump test with an IR test.
Ok, I'll replace the test with an IR-level test.
Apr 30 2019
Apr 23 2019
The implementation of getFlatAddressSpace in the AMDGPU backend does a bit more than just return a constant. This logic would need to be duplicated or the TTI called from their pass factory which would be the first use. TBH, I'm not too worried about the change to the factory breaking. This will be used in an out-of-tree project that tracks llvm fairly closely. If this breaks, I'll propose a change to exercise this from some in-tree code.
Apr 18 2019
But would it be possible to extract an .ll file generated from C++ (may be with some amendments) and run opt with the pass enabled?
This doesn't fix anything broken in LLVM but I did check that it didn't break any of the existing tests either :).
The pass is used by the AMDGPU and NVPTX backends. All their tests are passing with this change.
Apr 17 2019
Address review comments.
@arsenm Thanks for the review.