Anastasia added a comment.

Thanks for the comprehensive summary. I don't have many other suggestions in 
mind but I would like to add a few thoughts.

We could always consider writing the tests manually from scratch too. But it 
doesn't feel like a good cost/benefit trade-off.

Regarding 2 and 4 I think we should drive towards deprecation of `opencl-c.h` 
as it is a maintenance overhead but we could convert it into a test instead?

Aside from option 3 all other options will require adding non-standard testing 
formats in Clang. This means that if there is any core refactoring changes even 
outside of OpenCL affecting the functionality in the headers, it would be 
required to learn how we do the testing in order to address any breakages. 
Perhaps this can be mitigated by the documentation, but overall it is an extra 
burden to the community. Speaking from my personal experience I was in the 
situations breaking functionality outside of OpenCL and having a unified 
testing format is a big advantage in understanding and addressing the issues in 
unfamiliar code.

>   The output is big, currently already 56k lines, and I expect it will still 
> get a bit bigger when we add conditionals. I don't feel comfortable about 
> putting such big autogenerated files under git (regardless of whether we add 
> it as a single file, or as a collection of smaller files). Ideally, the repo 
> should only contain the source from which we generate things.

What problems do you see with checking in autogenerated files? There are some 
existing tests that check similar functionality and they seem to contain 
repetitive patterns. Honestly, I wouldn't be able to say if they were written 
with copy/paste or auto-generated but I am not sure whether it is very 
important. Of course, it would have been better if we have built up the 
functionality incrementally, and it is still an option. But it hasn't been 
possible in the past and as a matter of fact the original header was committed 
as one large file (around 15K lines).

>   Updates to OpenCLBuiltins.td will give large diffs when regenerating the 
> tests (regardless of whether the file is split or not).

Could you provide more details. I don't follow this.

>   Manually verifying that the generated test is actually correct is not 
> trivial.

I think the way to approach this would be to just assume that the test is 
correct and then fix any issues we discover as we go along with the 
development. There is nothing wrong with this though as even small manually 
written tests have bugs.


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

https://reviews.llvm.org/D97869

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to