Mail delivery failed: returning message to sender
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 3 10:48:24 PDT 2015
Any suggestion?
On Wed, Sep 2, 2015 at 11:09 AM, Jonathan Roelofs via llvm-commits <
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
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>
> --
> Jon Roelofs
> jonathan at codesourcery.com
> CodeSourcery / Mentor Embedded
> _______________________________________________
> 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/20150903/7d0094f4/attachment.html>
More information about the llvm-commits
mailing list