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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

FWIW, it seems like precommit CI is failing with problems in `x64 windows > Clang.SemaSYCL::accessors-targets.cpp` (which is largely hidden by the spam from the CI pipeline, unfortunately). Also, you should address the tidy warnings.



================
Comment at: clang/include/clang/Sema/Sema.h:12793
+  /// Contains generated OpenCL kernel functions for SYCL.
+  SmallVector<Decl *, 4> SYCLKernels;
+
----------------
Is 4 accurate? I would assume that 1 is more accurate as most people aren't going to be using SYCL at all.


================
Comment at: clang/include/clang/Sema/Sema.h:12798
+  /// Access to SYCL kernels.
+  SmallVectorImpl<Decl *> &getSYCLKernels() { return SYCLKernels; }
+
----------------
ABataev wrote:
> `ArrayRef<Decl *> getSYCLKernels()`
Also, this needs a const-correct overload (or surface only the const correct version, if possible).


================
Comment at: clang/lib/AST/ASTContext.cpp:10722
+  // If SYCL, only kernels are required.
+  if (LangOpts.SYCLIsDevice && !(D->hasAttr<OpenCLKernelAttr>()))
+    return false;
----------------



================
Comment at: clang/lib/Sema/SemaSYCL.cpp:45
+  /// accessor class.
+  static bool isSyclAccessorType(const QualType &Ty);
+
----------------
bader wrote:
> erichkeane wrote:
> > Isn't there a big rewrite going on downstream of these with `sycl_special_class`?  Why are we trying to upstream this before that happens?
> > Isn't there a big rewrite going on downstream of these with `sycl_special_class`?  
> 
> Yes.
> 
> > Why are we trying to upstream this before that happens?
> 
> The downstream work was initiated by this comment: https://reviews.llvm.org/D71016#inline-644645.
> This patch was uploaded for review here before refactoring work has started.
So should this patch be abandoned until that refactoring work completes, to avoid churn?


================
Comment at: clang/lib/Sema/SemaSYCL.cpp:100
+    if (Ref && Ref == MappingPair.first) {
+      auto NewDecl = MappingPair.second;
+      return DeclRefExpr::Create(
----------------
Please spell out the type as it's not spelled out in the initialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71016