[PATCH] D92004: [OpenCL] add CL 3.0 optional feature support to opencl-c.h

Mon Nov 30 10:57:21 PST 2020

Anastasia added a comment.

In D92004#2423441 <https://reviews.llvm.org/D92004#2423441>, @svenvh wrote:

>>> We probably don't want to hold up this patch until we have more thorough testing in place, since we don't even have any concrete plans for such testing at the moment.
>>
>> Well accepting such a significant change that implements the entire functionality of a new standard without a single test doesn't sound a workable approach. We should at least add some amount of testing in existing lib/Headers/opencl-c.h. However, instead of fighting with ad-hoc testing, it might save us time if we agree on a strategy and just do the testing properly. After all it is relatively clear what is to be tested. The issue is mainly with the testing time and amount of overloads to be tested. It should not be too difficult to guard such testing by a cmake option to avoid long execution time of default testing target of clang. However, it might be time-consuming to list all overloads. If we could use C++ templates it would help.
>
> Perhaps my assumption that we do not want to hold up this patch until we have more thorough testing in place was wrong then.  I agree it would be good to have better header testing of the header.  I merely wanted to point out that testing the header thoroughly is a substantial piece of work.  So if we want to have such testing in place first, we need to delay this patch and start a thread outside of this review.

Yes, this is a substantial amount of work indeed and yes a separate discussion would make sense. One way to approach this could be to start by testing the functionality affected by this patch only. Then we wouldn't necessarily need to hold off until we have the full testing which would take a while. We could split the features being added here into separate patches with tests being added accordingly to each of them.  I also think it will simplify the reviewing process and allow the key functionality to be added earlier instead of waiting for everything to be ready at once.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92004/new/

https://reviews.llvm.org/D92004