Mail delivery failed: returning message to sender
Jonathan Roelofs via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 3 10:50:32 PDT 2015
On 9/3/15 11:48 AM, Kostya Serebryany via llvm-commits wrote:
> Any suggestion?
d0k fixed this in r246694.
Cheers,
Jon
>
>
> On Wed, Sep 2, 2015 at 11:09 AM, Jonathan Roelofs via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
>
>
> On 9/2/15 12:07 PM, Mail Delivery System wrote:
>
> Forwarding to the new list...
>
>
>
> On 6/18/15 10:01 AM, Benjamin Kramer wrote:
>
> Author: d0k
> Date: Thu Jun 18 11:01:00 2015
> New Revision: 240023
>
> URL: http://llvm.org/viewvc/llvm-project?rev=240023&view=rev
> Log:
> [EliminateDuplicatePHINodes] Replace custom hash map with
> DenseSet.
>
> While there use hash_combine instead of hand-rolled hashing. No
> functionality change intended.
>
>
> Something about this patch is causing a crash in the attached
> testcase
> (which is already heavily reduced/redacted as much as I could). The
> failure mode is an assertion fail in DenseMap:
>
> Assertion failed: (!FoundVal && "Key already in new map?"), function
> moveFromOldBuckets, file
> /Users/jroelofs/workdir/svn/llvm2/include/llvm/ADT/DenseMap.h,
> line 301.
>
> Unfortunately I can't get this to reproduce on trunk, and my
> stabs at
> debugging it have been thus-far unsuccessful... Mind taking a look?
>
>
> Thanks,
>
> Jon
>
>
> Modified:
> llvm/trunk/lib/Transforms/Utils/Local.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=240023&r1=240022&r2=240023&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Thu Jun 18
> 11:01:00 2015
> @@ -14,6 +14,8 @@
>
> #include "llvm/Transforms/Utils/Local.h"
> #include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/DenseSet.h"
> +#include "llvm/ADT/Hashing.h"
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallPtrSet.h"
> #include "llvm/ADT/Statistic.h"
> @@ -828,64 +830,45 @@ bool llvm::TryToSimplifyUncondBranchFrom
> /// orders them so it usually won't matter.
> ///
> bool llvm::EliminateDuplicatePHINodes(BasicBlock *BB) {
> - bool Changed = false;
> -
> // This implementation doesn't currently consider
> undef operands
> // specially. Theoretically, two phis which are
> identical except for
> // one having an undef where the other doesn't could
> be collapsed.
>
> - // Map from PHI hash values to PHI nodes. If multiple
> PHIs have
> - // the same hash value, the element is the first PHI in the
> - // linked list in CollisionMap.
> - DenseMap<uintptr_t, PHINode *> HashMap;
> + struct PHIDenseMapInfo {
> + static PHINode *getEmptyKey() {
> + return DenseMapInfo<PHINode *>::getEmptyKey();
> + }
> + static PHINode *getTombstoneKey() {
> + return DenseMapInfo<PHINode *>::getTombstoneKey();
> + }
> + static unsigned getHashValue(PHINode *PN) {
> + // Compute a hash value on the operands. Instcombine
> will likely have
> + // sorted them, which helps expose duplicates, but we
> have to check all
> + // the operands to be safe in case instcombine hasn't
> run.
> + return static_cast<unsigned>(hash_combine(
> + hash_combine_range(PN->value_op_begin(),
> PN->value_op_end()),
> + hash_combine_range(PN->block_begin(),
> PN->block_end())));
> + }
> + static bool isEqual(PHINode *LHS, PHINode *RHS) {
> + if (LHS == getEmptyKey() || LHS == getTombstoneKey() ||
> + RHS == getEmptyKey() || RHS == getTombstoneKey())
> + return LHS == RHS;
> + return LHS->isIdenticalTo(RHS);
> + }
> + };
>
> - // Maintain linked lists of PHI nodes with common hash
> values.
> - DenseMap<PHINode *, PHINode *> CollisionMap;
> + // Set of unique PHINodes.
> + DenseSet<PHINode *, PHIDenseMapInfo> PHISet;
>
> // Examine each PHI.
> - for (BasicBlock::iterator I = BB->begin();
> - PHINode *PN = dyn_cast<PHINode>(I++); ) {
> - // Compute a hash value on the operands. Instcombine
> will likely have sorted
> - // them, which helps expose duplicates, but we have to
> check all the
> - // operands to be safe in case instcombine hasn't run.
> - uintptr_t Hash = 0;
> - // This hash algorithm is quite weak as hash functions
> go, but it seems
> - // to do a good enough job for this particular purpose,
> and is very quick.
> - for (User::op_iterator I = PN->op_begin(), E =
> PN->op_end(); I != E; ++I) {
> - Hash ^= reinterpret_cast<uintptr_t>(static_cast<Value
> *>(*I));
> - Hash = (Hash << 7) | (Hash >> (sizeof(uintptr_t) *
> CHAR_BIT - 7));
> - }
> - for (PHINode::block_iterator I = PN->block_begin(), E =
> PN->block_end();
> - I != E; ++I) {
> - Hash ^=
> reinterpret_cast<uintptr_t>(static_cast<BasicBlock *>(*I));
> - Hash = (Hash << 7) | (Hash >> (sizeof(uintptr_t) *
> CHAR_BIT - 7));
> - }
> - // Avoid colliding with the DenseMap sentinels ~0 and ~0-1.
> - Hash >>= 1;
> - // If we've never seen this hash value before, it's a
> unique PHI.
> - std::pair<DenseMap<uintptr_t, PHINode *>::iterator,
> bool> Pair =
> - HashMap.insert(std::make_pair(Hash, PN));
> - if (Pair.second) continue;
> - // Otherwise it's either a duplicate or a hash collision.
> - for (PHINode *OtherPN = Pair.first->second; ; ) {
> - if (OtherPN->isIdenticalTo(PN)) {
> - // A duplicate. Replace this PHI with its duplicate.
> - PN->replaceAllUsesWith(OtherPN);
> - PN->eraseFromParent();
> - Changed = true;
> - break;
> - }
> - // A non-duplicate hash collision.
> - DenseMap<PHINode *, PHINode *>::iterator I =
> CollisionMap.find(OtherPN);
> - if (I == CollisionMap.end()) {
> - // Set this PHI to be the head of the linked list
> of colliding PHIs.
> - PHINode *Old = Pair.first->second;
> - Pair.first->second = PN;
> - CollisionMap[PN] = Old;
> - break;
> - }
> - // Proceed to the next PHI in the list.
> - OtherPN = I->second;
> + bool Changed = false;
> + for (auto I = BB->begin(); PHINode *PN =
> dyn_cast<PHINode>(I++);) {
> + auto Inserted = PHISet.insert(PN);
> + if (!Inserted.second) {
> + // A duplicate. Replace this PHI with its duplicate.
> + PN->replaceAllUsesWith(*Inserted.first);
> + PN->eraseFromParent();
> + Changed = true;
> }
> }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> --
> Jon Roelofs
> jonathan at codesourcery.com <mailto:jonathan at codesourcery.com>
> CodeSourcery / Mentor Embedded
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded
More information about the llvm-commits
mailing list