mantognini added a comment.
Some relatively minor comments, overall direction seems good to me.
================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:67
public:
struct OpenCLOptionInfo {
// Option starts to be available in this OpenCL version
----------------
In OpenCLExtensions.def, the order is: pragma, avail, core, opt.
In this class it is: avail, core, opt, pragma (for both the attributes and the
constructor parameters).
There are very few usages of this, so the impact of this inconsistency is
minor. However, because the conversion between unsigned and bool is implicit an
error might go unnoticed for a long time.
================
Comment at: clang/lib/Basic/OpenCLOptions.cpp:89-91
+ OptMap.insert(
\
+ std::make_pair(llvm::StringRef{#Ext},
\
+ OpenCLOptionInfo{AvailVer, CoreVer, OptVer, Pragma}));
----------------
This could be done using `insert_or_assign(key, value)`.
================
Comment at: clang/lib/Basic/Targets.cpp:738
};
-#define OPENCL_GENERIC_EXTENSION(Ext, Avail, Core, Opt)
\
+#define OPENCL_GENERIC_EXTENSION(Ext, WithPragma, Avail, Core, Opt)
\
defineOpenCLExtMacro(#Ext, Avail, Core, Opt);
----------------
For consistency with OpenCLExtensions.def.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97052/new/
https://reviews.llvm.org/D97052
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
