[llvm] r259419 - SmallSet/SmallPtrSet: Refuse huge Small numbers

[Philip Reames via llvm-commits]

Having a cutoff seems entirely reasonable, but why 32?  Linear search 
performs surprisingly well and avoiding heap allocation can be 
valuable.  How'd you come up with the threshold chosen?

Philip

On 02/01/2016 02:05 PM, Matthias Braun via llvm-commits wrote:
> Author: matze
> Date: Mon Feb  1 16:05:16 2016
> New Revision: 259419
>
> URL: http://llvm.org/viewvc/llvm-project?rev=259419&view=rev
> Log:
> SmallSet/SmallPtrSet: Refuse huge Small numbers
>
> These sets do linear searching in small mode; It is not a good idea to
> use huge numbers as the small value here, save people from themselves by
> adding a static_assert.
>
> Differential Revision: http://reviews.llvm.org/D16706
>
> Modified:
>      llvm/trunk/include/llvm/ADT/SmallPtrSet.h
>      llvm/trunk/include/llvm/ADT/SmallSet.h
>      llvm/trunk/lib/Target/CppBackend/CPPBackend.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/SmallPtrSet.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=259419&r1=259418&r2=259419&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallPtrSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallPtrSet.h Mon Feb  1 16:05:16 2016
> @@ -329,6 +329,11 @@ public:
>   /// SmallPtrSetImplBase for details of the algorithm.
>   template<class PtrType, unsigned SmallSize>
>   class SmallPtrSet : public SmallPtrSetImpl<PtrType> {
> +  // 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.
> +  static_assert(SmallSize <= 32, "SmallSize should be small");
> +
>     typedef SmallPtrSetImpl<PtrType> BaseT;
>   
>     // Make sure that SmallSize is a power of two, round up if not.
>
> Modified: llvm/trunk/include/llvm/ADT/SmallSet.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallSet.h?rev=259419&r1=259418&r2=259419&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallSet.h Mon Feb  1 16:05:16 2016
> @@ -38,6 +38,11 @@ class SmallSet {
>     typedef typename SmallVector<T, N>::const_iterator VIterator;
>     typedef typename SmallVector<T, N>::iterator mutable_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.
> +  static_assert(N <= 32, "N should be small");
> +
>   public:
>     typedef size_t size_type;
>     SmallSet() {}
>
> Modified: llvm/trunk/lib/Target/CppBackend/CPPBackend.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/CppBackend/CPPBackend.cpp?rev=259419&r1=259418&r2=259419&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/CppBackend/CPPBackend.cpp (original)
> +++ llvm/trunk/lib/Target/CppBackend/CPPBackend.cpp Mon Feb  1 16:05:16 2016
> @@ -1651,8 +1651,8 @@ void CppWriter::printFunctionUses(const
>   
>     // Print type definitions for every type referenced by an instruction and
>     // make a note of any global values or constants that are referenced
> -  SmallPtrSet<GlobalValue*,64> gvs;
> -  SmallPtrSet<Constant*,64> consts;
> +  SmallPtrSet<GlobalValue*,32> gvs;
> +  SmallPtrSet<Constant*,32> consts;
>     for (Function::const_iterator BB = F->begin(), BE = F->end();
>          BB != BE; ++BB){
>       for (BasicBlock::const_iterator I = BB->begin(), E = BB->end();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits