Anastasia 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()) {
----------------
erik2020 wrote:
> 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?
I would say it is likely to be inlined in the optimized builds, removing
duplicate calls to find would be trickier but since it's part of the standard
libraries I think it is very likely going to happen.
Ok, if you are concerned about the performance we can leave it as is and only
update the comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90928/new/
https://reviews.llvm.org/D90928
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
