[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