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

Alexey Bataev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 6 06:35:47 PST 2020


ABataev added inline comments.


================
Comment at: clang/include/clang/Basic/LangOptions.h:122
 
+  enum class SYCLVersionList { SYCL_2015, SYCL_1_2_1 = SYCL_2015, undefined };
+
----------------
s/undefined/Undefined/g


================
Comment at: clang/include/clang/Basic/LangOptions.h:122
 
+  enum class SYCLVersionList { SYCL_2015, SYCL_1_2_1 = SYCL_2015, undefined };
+
----------------
ABataev wrote:
> s/undefined/Undefined/g
Do you use `SYCL_1_2_1` anywhere in the code? I don't see it is used and you can drop this enum.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2552-2553
+        llvm::StringSwitch<LangOptions::SYCLVersionList>(A->getValue())
+            .Cases("2015", "1.2.1", "121", "sycl-1.2.1",
+                   LangOptions::SYCLVersionList::SYCL_2015)
+            .Default(LangOptions::SYCLVersionList::undefined));
----------------
It does not match the list of values in `Options.td` file


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2562
+  } else if (Args.hasArg(options::OPT_fsycl_is_device) ||
+             Args.hasArg(options::OPT_fsycl)) {
+    Opts.setSYCLVersion(LangOptions::SYCLVersionList::SYCL_2015);
----------------
Can `OPT_fsycl` flag be ever passed to the frontend? Also, seems to me the driver has a special case already for `device` mode, so this code must be the dead code, actually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72857





More information about the cfe-commits mailing list