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

[Ronan Keryell via Phabricator via cfe-commits]

keryell added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:3419
+def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group<sycl_Group>, Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
+  HelpText<"SYCL language standard to compile for.">, Values<"2015, 121, 1.2.1, sycl-1.2.1">;
 
----------------
I suggest replacing all the 2015 by 2017.
While this is true SYCL 1.2 was published in 2015, SYCL 1.2.1 was published in 2017. Only 1.2.1 matters here since 1.2 was never fully implemented by any conformant implementation. https://en.wikipedia.org/wiki/SYCL


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4034
+      // Ensure the default version in SYCL mode is 1.2.1 (aka 2015)
+      CmdArgs.push_back("-sycl-std=2015");
+    }
----------------
Replace 2015 by 2017 in both lines above.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2554
+      Opts.SYCLVersion = llvm::StringSwitch<unsigned>(A->getValue())
+                             .Cases("2015", "1.2.1", "121", "sycl-1.2.1", 2015)
+                             .Default(0U);
----------------
Replace 2015 by 2017.


================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:456
+    // SYCL Version is set to a value when building SYCL applications
+    if (LangOpts.SYCLVersion == 2015)
+      Builder.defineMacro("CL_SYCL_LANGUAGE_VERSION", "121");
----------------
Replace 2015 by 2017.


================
Comment at: clang/test/Driver/sycl.c:5
+// RUN: %clang -### -fsycl -sycl-std=121 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=2015 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -fsycl -sycl-std=sycl-1.2.1 %s 2>&1 | FileCheck %s --check-prefix=ENABLED
----------------
Replace all the 2015 by 2017 here and below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857