[PATCH] D72857: [SYCL] Driver option to enable SYCL mode and select SYCL version

Wed Jan 22 05:02:09 PST 2020

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