ABataev added inline comments.

================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:11320
+  bool MutexClauseEncountered = false;
+  llvm::SmallSet<OpenMPClauseKind, 2> EncounteredAtomicKinds;
   for (const OMPClause *C : Clauses) {
----------------
tianshilei1992 wrote:
> ABataev wrote:
> > tianshilei1992 wrote:
> > > ABataev wrote:
> > > > tianshilei1992 wrote:
> > > > > ABataev wrote:
> > > > > > SmallVector with 2 elements should be enough, no need to use 
> > > > > > SmallSet here.
> > > > > `SmallVector` doesn't provide `contains`, which we use to check if 
> > > > > `OMPC_compare` and `OMPC_capture` exist at the same time. Using 
> > > > > `SmallVector` requires us to have a complicated comparison, for 
> > > > > example:
> > > > > ```
> > > > > (EncounteredAtomicKinds[0] == OMPC_compare && 
> > > > > EncounteredAtomicKinds[1] == OMPC_capture) || 
> > > > > (EncounteredAtomicKinds[1] == OMPC_compare && 
> > > > > EncounteredAtomicKinds[0] == OMPC_capture)
> > > > > ```
> > > > > which is not as neat as `SmallSet`.
> > > > Use llvm::find
> > > That is linear time. Why it is better than `SmallSet`?
> > Because there are only 2 elements, no?
> It could have more. If user writes it like `atomic compare compare capture 
> capture capture`, then it will have 5 elements. `SmallSet<T>::insert` returns 
> a `pair` and we know if it is already in the set, which I have not added it 
> yet but I'll. `SmallVector` then is not that easy to use. Of course you could 
> say do `llvm::find` beforehand but it's not neat.
Is this correct at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

Reply via email to