[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

Ronan Keryell via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 10 18:09:52 PST 2019


keryell added inline comments.


================
Comment at: clang/lib/Sema/SemaSYCL.cpp:44
+
+class KernelBodyTransform : public TreeTransform<KernelBodyTransform> {
+public:
----------------
Feel free to add more comments in this area up to line 103.


================
Comment at: clang/lib/Sema/SemaSYCL.cpp:417
+      Util::DeclContextDesc{clang::Decl::Kind::ClassTemplateSpecialization,
+                            "accessor"}};
+  return matchQualifiedTypeName(Ty, Scopes);
----------------
After more thought, perhaps the solution proposed by Victor Lomüller during the last SYCL upstreaming meeting about marking accessor classes with some attribute instead of detecting concrete type names is a better generic approach.
I am more convinced now by his argument allowing more experimenting with alternative runtimes, since it looks like it is the only place we detect type names. For example the kernels are marked with an attribute in the runtime instead of concretely detecting the `parallel_for()` functions and so on.


================
Comment at: clang/test/CodeGenSYCL/device-functions.cpp:2
+// RUN: %clang_cc1 -triple spir64 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template <typename T>
----------------
Missing description about the purpose of this test


================
Comment at: clang/test/SemaSYCL/fake-accessors.cpp:2
+// RUN: %clang_cc1 -I %S/Inputs -fsycl-is-device -ast-dump %s | FileCheck %s
+
+#include <sycl.hpp>
----------------
Missing description about the purpose of this test.
I am struggling about understanding what this test is for...
OK, after coming back later, I think I got it. I was confused by the fact that in the kernels you are using both true accessors (A, B & C) *and* some objects with names similar to SYCL accessor.
Is it possible to have some tests without true accessors?


================
Comment at: clang/test/SemaSYCL/mangle-kernel.cpp:3
+// RUN: %clang_cc1 -triple spir-unknown-unknown-unknown -I %S/Inputs -I %S/../Headers/Inputs/include/ -fsycl-is-device -ast-dump %s | FileCheck %s --check-prefix=CHECK-32
+#include <sycl.hpp>
+#include <stdlib.h>
----------------
Missing description about the purpose of this test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71016





More information about the cfe-commits mailing list