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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to