Anastasia added a comment.

I think this is a very good starting point for the testing of built-in 
functions. Btw should we now remove '[Draft]' from the title?



================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:242
+  // functions.
+  void Emit();
+
----------------
Should be `emit` to adhere to coding style?


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:969
+
+void OpenCLBuiltinTestEmitter::getTypeLists(
+    Record *Type, TypeFlags &Flags, std::vector<Record *> &TypeList,
----------------
I would add a documentation for this function, it is not very obvious.


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1004
+    SmallVectorImpl<SmallVector<std::string, 2>> &Types) {
+  // Find out if there are any GenTypes in this signature, and if so, calculate
+  // into how many signatures they will expand.
----------------
Maybe this should be lifted as documentation of the header of this function?


================
Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:1033
+    for (unsigned ArgNum = 0; ArgNum < Signature.size(); ArgNum++) {
+      // For differently-sized GenTypes in a parameter list, the smaller
+      // GenTypes just repeat.
----------------
I don't get this comment and its exact reference to the code.


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
  • [PATCH] D97869: [OpenCL... Sven van Haastregt via Phabricator via cfe-commits
    • [PATCH] D97869: [O... Anastasia Stulova via Phabricator via cfe-commits

Reply via email to