[llvm] r333601 - [NewGVN] Fix set comparison; reflow comment

[George Burgess IV via llvm-commits]

(Not overly familiar with NewGVN, but should we also be comparing the
contents of MemoryMembers in this?)

On Wed, May 30, 2018 at 3:28 PM George Burgess IV via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: gbiv
> Date: Wed May 30 15:24:08 2018
> New Revision: 333601
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333601&view=rev
> Log:
> [NewGVN] Fix set comparison; reflow comment
>
> Looks like we intended to compare this->Members with Other->Members
> here, but ended up comparing this->Members with this->Members. Oops. :)
>
> Since CongruenceClass::Members is a SmallPtrSet anyway, we can probably
> skip building std::sets if we're willing to write a bit more code.
>
> This appears to be no functional change (for sufficiently lax values of
> "no"): this equality check was only being called inside of an assert.
> So, worst case, we'll catch more bugs in the form of assertion failures.
>
> Thanks to d0k for noting this!
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
>
> Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=333601&r1=333600&r2=333601&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Wed May 30 15:24:08 2018
> @@ -366,9 +366,8 @@ public:
>    // True if this class has no memory members.
>    bool definesNoMemory() const { return StoreCount == 0 &&
> memory_empty(); }
>
> -  // Return true if two congruence classes are equivalent to each other.
> This
> -  // means
> -  // that every field but the ID number and the dead field are equivalent.
> +  // Return true if two congruence classes are equivalent to each other.
> This
> +  // means that every field but the ID number and the dead field are
> equivalent.
>    bool isEquivalentTo(const CongruenceClass *Other) const {
>      if (!Other)
>        return false;
> @@ -383,10 +382,12 @@ public:
>        if (!DefiningExpr || !Other->DefiningExpr ||
>            *DefiningExpr != *Other->DefiningExpr)
>          return false;
> -    // We need some ordered set
> -    std::set<Value *> AMembers(Members.begin(), Members.end());
> -    std::set<Value *> BMembers(Members.begin(), Members.end());
> -    return AMembers == BMembers;
> +
> +    if (Members.size() != Other->Members.size())
> +      return false;
> +
> +    return all_of(Members,
> +                  [&](const Value *V) { return Other->Members.count(V);
> });
>    }
>
>  private:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180530/ef5f35b7/attachment.html>