https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/109412
>From 58c88d6fee0f1aa486201189bbe67cea7da2d25e Mon Sep 17 00:00:00 2001 From: Victor Campos <victor.cam...@arm.com> Date: Fri, 9 Aug 2024 13:51:52 +0100 Subject: [PATCH 1/2] [ADT] Style and nit fixes in SmallSet --- llvm/include/llvm/ADT/SmallSet.h | 43 ++++++++++++++++---------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h index a16e8ac6f07552..630c98504261aa 100644 --- a/llvm/include/llvm/ADT/SmallSet.h +++ b/llvm/include/llvm/ADT/SmallSet.h @@ -46,24 +46,24 @@ class SmallSetIterator VecIterTy VecIter; }; - bool isSmall; + bool IsSmall; public: - SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), isSmall(false) {} + SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), IsSmall(false) {} - SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), isSmall(true) {} + SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), IsSmall(true) {} // Spell out destructor, copy/move constructor and assignment operators for // MSVC STL, where set<T>::const_iterator is not trivially copy constructible. ~SmallSetIterator() { - if (isSmall) + if (IsSmall) VecIter.~VecIterTy(); else SetIter.~SetIterTy(); } - SmallSetIterator(const SmallSetIterator &Other) : isSmall(Other.isSmall) { - if (isSmall) + SmallSetIterator(const SmallSetIterator &Other) : IsSmall(Other.IsSmall) { + if (IsSmall) VecIter = Other.VecIter; else // Use placement new, to make sure SetIter is properly constructed, even @@ -71,8 +71,8 @@ class SmallSetIterator new (&SetIter) SetIterTy(Other.SetIter); } - SmallSetIterator(SmallSetIterator &&Other) : isSmall(Other.isSmall) { - if (isSmall) + SmallSetIterator(SmallSetIterator &&Other) : IsSmall(Other.IsSmall) { + if (IsSmall) VecIter = std::move(Other.VecIter); else // Use placement new, to make sure SetIter is properly constructed, even @@ -83,11 +83,11 @@ class SmallSetIterator SmallSetIterator& operator=(const SmallSetIterator& Other) { // Call destructor for SetIter, so it gets properly destroyed if it is // not trivially destructible in case we are setting VecIter. - if (!isSmall) + if (!IsSmall) SetIter.~SetIterTy(); - isSmall = Other.isSmall; - if (isSmall) + IsSmall = Other.IsSmall; + if (IsSmall) VecIter = Other.VecIter; else new (&SetIter) SetIterTy(Other.SetIter); @@ -97,11 +97,11 @@ class SmallSetIterator SmallSetIterator& operator=(SmallSetIterator&& Other) { // Call destructor for SetIter, so it gets properly destroyed if it is // not trivially destructible in case we are setting VecIter. - if (!isSmall) + if (!IsSmall) SetIter.~SetIterTy(); - isSmall = Other.isSmall; - if (isSmall) + IsSmall = Other.IsSmall; + if (IsSmall) VecIter = std::move(Other.VecIter); else new (&SetIter) SetIterTy(std::move(Other.SetIter)); @@ -109,22 +109,22 @@ class SmallSetIterator } bool operator==(const SmallSetIterator &RHS) const { - if (isSmall != RHS.isSmall) + if (IsSmall != RHS.IsSmall) return false; - if (isSmall) + if (IsSmall) return VecIter == RHS.VecIter; return SetIter == RHS.SetIter; } SmallSetIterator &operator++() { // Preincrement - if (isSmall) - VecIter++; + if (IsSmall) + ++VecIter; else - SetIter++; + ++SetIter; return *this; } - const T &operator*() const { return isSmall ? *VecIter : *SetIter; } + const T &operator*() const { return IsSmall ? *VecIter : *SetIter; } }; /// SmallSet - This maintains a set of unique values, optimizing for the case @@ -167,9 +167,8 @@ class SmallSet { if (isSmall()) { // Since the collection is small, just do a linear search. return vfind(V) == Vector.end() ? 0 : 1; - } else { - return Set.count(V); } + return Set.count(V); } /// insert - Insert an element into the set if it isn't already there. >From 3d83c5456c35f891aefa65f7cc6b37795af99c32 Mon Sep 17 00:00:00 2001 From: Victor Campos <victor.cam...@arm.com> Date: Fri, 20 Sep 2024 11:43:18 +0100 Subject: [PATCH 2/2] [ADT] Simplify SmallSet - Remove dependence on `STLExtras.h`. - Remove unused header inclusions. - Make `count` use `contains` for deduplication. - Replace hand-written linear scans on Vector by `std::find`. --- clang/lib/Basic/TargetID.cpp | 1 + llvm/include/llvm/ADT/SmallSet.h | 37 +++++++------------------------- 2 files changed, 9 insertions(+), 29 deletions(-) diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp index 3c06d9bad1dc0d..fa1bfec2aacb9c 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/TargetID.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/TargetParser.h" diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h index 630c98504261aa..8d7511bf0bc8d9 100644 --- a/llvm/include/llvm/ADT/SmallSet.h +++ b/llvm/include/llvm/ADT/SmallSet.h @@ -16,14 +16,10 @@ #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/iterator.h" -#include "llvm/Support/Compiler.h" -#include "llvm/Support/type_traits.h" #include <cstddef> #include <functional> #include <set> -#include <type_traits> #include <utility> namespace llvm { @@ -139,10 +135,6 @@ class SmallSet { SmallVector<T, N> Vector; std::set<T, C> Set; - using VIterator = typename SmallVector<T, N>::const_iterator; - using SIterator = typename std::set<T, C>::const_iterator; - using mutable_iterator = typename SmallVector<T, N>::iterator; - // In small mode SmallPtrSet uses linear search for the elements, so it is // not a good idea to choose this value too high. You may consider using a // DenseSet<> instead if you expect many elements in the set. @@ -163,13 +155,7 @@ class SmallSet { } /// count - Return 1 if the element is in the set, 0 otherwise. - size_type count(const T &V) const { - if (isSmall()) { - // Since the collection is small, just do a linear search. - return vfind(V) == Vector.end() ? 0 : 1; - } - return Set.count(V); - } + size_type count(const T &V) const { return contains(V) ? 1 : 0; } /// insert - Insert an element into the set if it isn't already there. /// Returns a pair. The first value of it is an iterator to the inserted @@ -181,7 +167,7 @@ class SmallSet { return std::make_pair(const_iterator(I), Inserted); } - VIterator I = vfind(V); + auto I = std::find(Vector.begin(), Vector.end(), V); if (I != Vector.end()) // Don't reinsert if it already exists. return std::make_pair(const_iterator(I), false); if (Vector.size() < N) { @@ -206,11 +192,11 @@ class SmallSet { bool erase(const T &V) { if (!isSmall()) return Set.erase(V); - for (mutable_iterator I = Vector.begin(), E = Vector.end(); I != E; ++I) - if (*I == V) { - Vector.erase(I); - return true; - } + auto I = std::find(Vector.begin(), Vector.end(), V); + if (I != Vector.end()) { + Vector.erase(I); + return true; + } return false; } @@ -234,19 +220,12 @@ class SmallSet { /// Check if the SmallSet contains the given element. bool contains(const T &V) const { if (isSmall()) - return vfind(V) != Vector.end(); + return std::find(Vector.begin(), Vector.end(), V) != Vector.end(); return Set.find(V) != Set.end(); } private: bool isSmall() const { return Set.empty(); } - - VIterator vfind(const T &V) const { - for (VIterator I = Vector.begin(), E = Vector.end(); I != E; ++I) - if (*I == V) - return I; - return Vector.end(); - } }; /// If this set is of pointer values, transparently switch over to using _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits