Mail delivery failed: returning message to sender
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 3 10:51:27 PDT 2015
Oh, cool, thanks!
On Thu, Sep 3, 2015 at 10:50 AM, Jonathan Roelofs <jroelofs.lists at gmail.com>
wrote:
>
>
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150903/527b3b06/attachment.html>
More information about the llvm-commits
mailing list