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