[PATCH] D97869: [OpenCL][Draft] Add OpenCL builtin test generator

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D97869#2628679 <https://reviews.llvm.org/D97869#2628679>, @azabaznov wrote:

> I have one more though.
>
> I like the idea of turning `opencl-c.h` into the test: as it is already in the repo and is already being used for quite a while we can assume it as a mainline for now. I think the first step should be to test that `-fdeclare-oprencl-builtins` generates the same built-ins which are declared in `opencl-c.h`. If we can't do this //programmable way// we can use AST pretty-printing for tablegen header and `opencl-c.h` and compare these with //diff //(we can also do a clean-up of AST files with //rm// while running that tests).
>
> Once this is done we can incrementally modify either `opencl-c.h` header and tablegen header. Testing changes to either of them can combine sanity check as @svenvh suggested in **builtins-opencl2.0.check** and diff for AST pretty printing.
>
> Advantages:
>
> - Time of testing. Locally I get nice results: 
>
>   $ time ./build_release/bin/clang-check -extra-arg=-cl-std=CL2.0 --ast-print  llvm-project/clang/lib/Headers/opencl-c.h &> header.h
>   real    0m0.182s
>   user    0m0.162s
>   sys     0m0.020s
>
>   But not yet clear how much time such printing will take for tablegen header. I assume it won't take a lot longer.
>
> - This will keep changes to `opencl-h` header and tablegen header consistent (until `opencl-c.h` will be deprecated)
>
>  
> Disadvantages:
>
> - Still doesn't eliminate subtle errors, need to carefully collect information from the spec about amount of the built-ins

Thanks for elaborating on this option. Yes, I think this could make the testing quick. The biggest concern with this I see is that it doesn't actually show that the header is working through the clang frontend. For example, we could easily change something very significant like remove the definition of macros guarding the function declarations and not notice that it means the declarations are no longer available to the users of clang. So I would say while it covers all the builtin overloads it doesn't give the confidence that them at all are exposed correctly to the users. We could of course work around this by providing complimentary partial testing using clang invocation but it means our testing will become harder to understand.

Overall I think we have made a good progress on brainstorming here so I suggest to give the ideas broader visibility via RFC to cfe-dev. We could include the following:

- Start from standard clang testing idea i.e. adding a large test file(s) using clang invocation and calling the functions (either written manually or autogenerated).
- Summarize the issues with the standard approach and highlight that similar tests with other clang features (i.e. target intrinsics) show time bottlenecks.
- Enumerate various alternative options and hybrid options to avoid adding large test files with long execution time highlighting adv/disadv for each of them.

Does this sound reasonable? Is there anything else we should include in such RFC?


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

https://reviews.llvm.org/D97869