[llvm-commits] [llvm] r152300 - in /llvm/trunk: include/llvm/ADT/FoldingSet.h lib/Support/FoldingSet.cpp

[Benjamin Kramer]

On 08.03.2012, at 08:50, Daniel Dunbar <daniel at zuster.org> wrote:

> Author: ddunbar
> Date: Thu Mar  8 01:42:18 2012
> New Revision: 152300
>
> URL: http://llvm.org/viewvc/llvm-project?rev=152300&view=rev
> Log:
> [ADT] Change the trivial FoldingSetNodeID::Add* methods to be inline, reapplied
> with a fix for the longstanding over-read of 32-bit pointer values.

I don't see how the old code would over-read on a 32 bit pointer.
append(&Ptr, &Ptr+1) is just a funky way to write push_back when ptr
has the same size as the elements in the vector. Remember that this
method is very hot, so not calling push_back twice is goodness.

- Ben
>
> Modified:
>    llvm/trunk/include/llvm/ADT/FoldingSet.h
>    llvm/trunk/lib/Support/FoldingSet.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/FoldingSet.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=152300&r1=152299&r2=152300&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/FoldingSet.h Thu Mar  8 01:42:18 2012
> @@ -17,6 +17,7 @@
> #define LLVM_ADT_FOLDINGSET_H
>
> #include "llvm/Support/DataTypes.h"
> +#include "llvm/Support/ErrorHandling.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/ADT/StringRef.h"
>
> @@ -310,6 +311,7 @@
>   void AddInteger(unsigned long long I);
>   void AddBoolean(bool B) { AddInteger(B ? 1U : 0U); }
>   void AddString(StringRef String);
> +  /// AddNodeID - Adds the Bit data of another ID to *this.
>   void AddNodeID(const FoldingSetNodeID &ID);
>
>   template <typename T>
> @@ -675,6 +677,55 @@
>     ID.AddPointer(X);
>   }
> };
> +
> +//===----------------------------------------------------------------------===//
> +// FoldingSetNodeID Inline function definitions
> +
> +/// Add* - Add various data types to Bit data.
> +///
> +inline void FoldingSetNodeID::AddPointer(const void *Ptr) {
> +  // Note: this adds pointers to the hash using sizes and endianness that
> +  // depend on the host.  It doesn't matter however, because hashing on
> +  // pointer values in inherently unstable.  Nothing  should depend on the
> +  // ordering of nodes in the folding set.
> +  if (sizeof(void*) == sizeof(unsigned))
> +    AddInteger((unsigned) (unsigned long long) Ptr);
> +  else if (sizeof(void*) == sizeof(unsigned long long)) {
> +    AddInteger((unsigned long long) Ptr);
> +  } else {
> +    llvm_unreachable("unexpected sizeof(void*)");
> +  }
> +}
> +inline void FoldingSetNodeID::AddInteger(signed I) {
> +  Bits.push_back(I);
> +}
> +inline void FoldingSetNodeID::AddInteger(unsigned I) {
> +  Bits.push_back(I);
> +}
> +inline void FoldingSetNodeID::AddInteger(long I) {
> +  AddInteger((unsigned long)I);
> +}
> +inline void FoldingSetNodeID::AddInteger(unsigned long I) {
> +  if (sizeof(long) == sizeof(int))
> +    AddInteger(unsigned(I));
> +  else if (sizeof(long) == sizeof(long long)) {
> +    AddInteger((unsigned long long)I);
> +  } else {
> +    llvm_unreachable("unexpected sizeof(long)");
> +  }
> +}
> +inline void FoldingSetNodeID::AddInteger(long long I) {
> +  AddInteger((unsigned long long)I);
> +}
> +inline void FoldingSetNodeID::AddInteger(unsigned long long I) {
> +  AddInteger(unsigned(I));
> +  if ((uint64_t)(unsigned)I != I)
> +    Bits.push_back(unsigned(I >> 32));
> +}
> +inline void FoldingSetNodeID::AddNodeID(const FoldingSetNodeID &ID) {
> +  Bits.append(ID.Bits.begin(), ID.Bits.end());
> +}
> +
> } // End of namespace llvm.
>
> #endif
>
> Modified: llvm/trunk/lib/Support/FoldingSet.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=152300&r1=152299&r2=152300&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/FoldingSet.cpp (original)
> +++ llvm/trunk/lib/Support/FoldingSet.cpp Thu Mar  8 01:42:18 2012
> @@ -41,43 +41,6 @@
> //===----------------------------------------------------------------------===//
> // FoldingSetNodeID Implementation
>
> -/// Add* - Add various data types to Bit data.
> -///
> -void FoldingSetNodeID::AddPointer(const void *Ptr) {
> -  // Note: this adds pointers to the hash using sizes and endianness that
> -  // depend on the host.  It doesn't matter however, because hashing on
> -  // pointer values in inherently unstable.  Nothing  should depend on the
> -  // ordering of nodes in the folding set.
> -  Bits.append(reinterpret_cast<unsigned *>(&Ptr),
> -              reinterpret_cast<unsigned *>(&Ptr+1));
> -}
> -void FoldingSetNodeID::AddInteger(signed I) {
> -  Bits.push_back(I);
> -}
> -void FoldingSetNodeID::AddInteger(unsigned I) {
> -  Bits.push_back(I);
> -}
> -void FoldingSetNodeID::AddInteger(long I) {
> -  AddInteger((unsigned long)I);
> -}
> -void FoldingSetNodeID::AddInteger(unsigned long I) {
> -  if (sizeof(long) == sizeof(int))
> -    AddInteger(unsigned(I));
> -  else if (sizeof(long) == sizeof(long long)) {
> -    AddInteger((unsigned long long)I);
> -  } else {
> -    llvm_unreachable("unexpected sizeof(long)");
> -  }
> -}
> -void FoldingSetNodeID::AddInteger(long long I) {
> -  AddInteger((unsigned long long)I);
> -}
> -void FoldingSetNodeID::AddInteger(unsigned long long I) {
> -  AddInteger(unsigned(I));
> -  if ((uint64_t)(unsigned)I != I)
> -    Bits.push_back(unsigned(I >> 32));
> -}
> -
> void FoldingSetNodeID::AddString(StringRef String) {
>   unsigned Size =  String.size();
>   Bits.push_back(Size);
> @@ -129,12 +92,7 @@
>   Bits.push_back(V);
> }
>
> -// AddNodeID - Adds the Bit data of another ID to *this.
> -void FoldingSetNodeID::AddNodeID(const FoldingSetNodeID &ID) {
> -  Bits.append(ID.Bits.begin(), ID.Bits.end());
> -}
> -
> -/// ComputeHash - Compute a strong hash value for this FoldingSetNodeID, used to
> +/// ComputeHash - Compute a strong hash value for this FoldingSetNodeID, used to
> /// lookup the node in the FoldingSetImpl.
> unsigned FoldingSetNodeID::ComputeHash() const {
>   return FoldingSetNodeIDRef(Bits.data(), Bits.size()).ComputeHash();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits