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

[Chandler Carruth]

On Fri, Sep 7, 2012 at 9:25 PM, Ted Kremenek <kremenek at apple.com> wrote:

> Author: kremenek
> Date: Fri Sep  7 23:25:29 2012
> New Revision: 163454
>
> URL: http://llvm.org/viewvc/llvm-project?rev=163454&view=rev
> Log:
> Add operator< for FoldingSetNodeID.
>

While I'm good with adding this if there are good use cases (such as places
where a sort-based container is more efficient), I think it needs comments:
the order isn't really any more deterministic than the folding set's order.
See my reply to your commit using this for more here.

In the general case, we have lots of pointers that are inputs to a
FoldingSetNodeID. The hashing function in use doesn't guarantee any
stability from run-to-run either. While the current implementation happens
to be stable from run-to-run, I've proposed and would still like to
implement a change that ensures each execution is seeded differently so
that inherently non-deterministic behavior is discovered early and reliably
rather than late and sometimes only on systems with ASLR.


>
> 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=163454&r1=163453&r2=163454&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/FoldingSet.h Fri Sep  7 23:25:29 2012
> @@ -278,6 +278,10 @@
>
>    bool operator==(FoldingSetNodeIDRef) const;
>
> +  /// Used to compare the "ordering" of two nodes as defined by the
> +  /// profiled bits and their ordering defined by memcmp().
> +  bool operator<(FoldingSetNodeIDRef) const;
> +
>    const unsigned *getData() const { return Data; }
>    size_t getSize() const { return Size; }
>  };
> @@ -327,6 +331,11 @@
>    bool operator==(const FoldingSetNodeID &RHS) const;
>    bool operator==(const FoldingSetNodeIDRef RHS) const;
>
> +  /// Used to compare the "ordering" of two nodes as defined by the
> +  /// profiled bits and their ordering defined by memcmp().
> +  bool operator<(const FoldingSetNodeID &RHS) const;
> +  bool operator<(const FoldingSetNodeIDRef RHS) const;
> +
>    /// Intern - Copy this node's data to a memory region allocated from the
>    /// given allocator and return a FoldingSetNodeIDRef describing the
>    /// interned data.
>
> Modified: llvm/trunk/lib/Support/FoldingSet.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=163454&r1=163453&r2=163454&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/FoldingSet.cpp (original)
> +++ llvm/trunk/lib/Support/FoldingSet.cpp Fri Sep  7 23:25:29 2012
> @@ -38,6 +38,14 @@
>    return memcmp(Data, RHS.Data, Size*sizeof(*Data)) == 0;
>  }
>
> +/// Used to compare the "ordering" of two nodes as defined by the
> +/// profiled bits and their ordering defined by memcmp().
> +bool FoldingSetNodeIDRef::operator<(FoldingSetNodeIDRef RHS) const {
> +  if (Size != RHS.Size)
> +    return Size < RHS.Size;
> +  return memcmp(Data, RHS.Data, Size*sizeof(*Data)) < 0;
> +}
> +
>
>  //===----------------------------------------------------------------------===//
>  // FoldingSetNodeID Implementation
>
> @@ -152,6 +160,16 @@
>    return FoldingSetNodeIDRef(Bits.data(), Bits.size()) == RHS;
>  }
>
> +/// Used to compare the "ordering" of two nodes as defined by the
> +/// profiled bits and their ordering defined by memcmp().
> +bool FoldingSetNodeID::operator<(const FoldingSetNodeID &RHS)const{
> +  return *this < FoldingSetNodeIDRef(RHS.Bits.data(), RHS.Bits.size());
> +}
> +
> +bool FoldingSetNodeID::operator<(FoldingSetNodeIDRef RHS) const {
> +  return FoldingSetNodeIDRef(Bits.data(), Bits.size()) < RHS;
> +}
> +
>  /// Intern - Copy this node's data to a memory region allocated from the
>  /// given allocator and return a FoldingSetNodeIDRef describing the
>  /// interned data.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120908/f7e4843f/attachment.html>