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