Ruyk added inline comments.
================ 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") ---------------- bader wrote: > 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? Thats a good point. I don't see strong reasons for the enum, basically I read the comment in https://code.woboq.org/llvm/clang/include/clang/Basic/LangOptions.def.html#22 ``` // ENUM_LANGOPT: for options that have enumeration, rather than unsigned, type. ``` And since there is a known set of SYCL specifications, made more sense to enumerate it. It also simplifies writing variants to 1.2.1 (e.g. 1.2.1-oneapi) in the code since then you can add another entry to the enum. But no strong feelings, so feel free to change it. ================ 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">; ---------------- bader wrote: > 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. In the case of SYCL, you may want to compile your code with C++17 and SYCL 2015, in which case you need both -std=c++17 and -sycl=sycl-2015 . SYCL specification mandates a minimum C++ version but users can write code on newer versions as long as the code in the kernel scope is still valid. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3989 + // Ensure the default version in SYCL mode is 1.2.1 + CmdArgs.push_back("-sycl-std=1.2.1"); + } ---------------- This should probably change to 2015 if we are allowing that, just for consistency 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
