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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits