[PATCH] D90928: [OpenCL] Check for extension string extension lookup

Erik Tomusk via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 23 09:41:31 PST 2020


erik2020 added inline comments.


================
Comment at: clang/include/clang/Basic/OpenCLOptions.h:47
   bool isSupported(llvm::StringRef Ext, const LangOptions &LO) const {
+    auto E = OptMap.find(Ext);
+    if (E == OptMap.end()) {
----------------
Anastasia wrote:
> Btw how about we use `isKnown` instead because it does exactly that? Also, I think we should update the comment to explain this change in the API behavior and add a comment for `isKnown`. 
My thinking was that this way, there's only one look-up in `OptMap`, but when using `isKnown()` there are two. I don't know how good a compiler would be at inlining `isKnown()` and de-duplicating the look-ups.

But maybe the overhead doesn't really matter in this case and calling `isKnown()` is clearer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90928



More information about the cfe-commits mailing list