bader marked 2 inline comments as done. bader added a comment. > Maybe we should use the year of issue (2015 instead of 1.2.1) for the > -sycl-std version? That would be more stable for the upcoming SYCL versions, > and match somehow the C++ versioning.
Sounds good to me. I'll update the patch. I also have a couple of design related questions below. ================ Comment at: clang/include/clang/Basic/LangOptions.def:206 LANGOPT(OpenCLCPlusPlusVersion , 32, 0, "C++ for OpenCL version") +ENUM_LANGOPT(SYCLVersion, SYCLVersionList, 4, SYCLVersionList::undefined, "Version of the SYCL standard used") LANGOPT(NativeHalfType , 1, 0, "Native half type support") ---------------- All other language options controlling standard versions are added as "LANGOPT" i.e. `int`. Why SYCLVersion is different? @Ruyk, do you think we should convert other options (e.g. `OpenCL`) to enums as well? ================ Comment at: clang/include/clang/Driver/Options.td:3401 +def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group<sycl_Group>, Flags<[CC1Option]>, + HelpText<"SYCL language standard to compile for.">, Values<"1.2.1">; ---------------- What do you think we integrate sycl versions to existing clang options controlling language version: `-std`. As far as I can see it's used for all the C/C+ extensions like OpenMP/OpenCL/CUDA/HIP/ObjC. If I understand correctly clang supports `-cl-std` only because it's required by OpenCL standard. Similar option (i.e. `-sycl-std`) is not required by the SYCL specification, so using `-std` is more aligned with existing clang design. See clang/include/clang/Basic/LangStandard.h and clang/include/clang/Basic/LangStandards.def. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72857/new/ https://reviews.llvm.org/D72857 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
