<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Thanks!<div class="">-Chris<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Apr 21, 2020, at 2:41 AM, Craig Topper <<a href="mailto:craig.topper@gmail.com" class="">craig.topper@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">This was causing some failures of the assert in operator*/operator-> in the iterator for me. I believe I've fixed the issue in 2cf3c033f3a1d4c2c36a94a80b7ba6bcba1e8711<div class=""><br class=""></div><div class="">~Craig<br class=""><div class=""><br class=""></div></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 20, 2020 at 5:31 PM Chris Bieneman via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br class="">
Author: Chris Bieneman<br class="">
Date: 2020-04-20T19:31:32-05:00<br class="">
New Revision: 887efa51c1e0e43ca684ed78b92dbc3a0720881b<br class="">
<br class="">
URL: <a href="https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b" rel="noreferrer" target="_blank" class="">https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b</a><br class="">
DIFF: <a href="https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b.diff" rel="noreferrer" target="_blank" class="">https://github.com/llvm/llvm-project/commit/887efa51c1e0e43ca684ed78b92dbc3a0720881b.diff</a><br class="">
<br class="">
LOG: Fix DenseMap iterator asserts when shouldReverseIterate==true<br class="">
<br class="">
This patch gets the asserts working correctly when LLVM_REVERSE_ITERATION=On by fixing the iterators returned by the DenseMap::find* methods so that they return well-formed iterators that work with reverse iteration, and satisfy the assertions.<br class="">
<br class="">
Added: <br class="">
<br class="">
<br class="">
Modified: <br class="">
llvm/include/llvm/ADT/DenseMap.h<br class="">
<br class="">
Removed: <br class="">
<br class="">
<br class="">
<br class="">
################################################################################<br class="">
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h<br class="">
index f453ce539e2b..ff9dab15291f 100644<br class="">
--- a/llvm/include/llvm/ADT/DenseMap.h<br class="">
+++ b/llvm/include/llvm/ADT/DenseMap.h<br class="">
@@ -149,13 +149,19 @@ class DenseMapBase : public DebugEpochBase {<br class="">
iterator find(const_arg_type_t<KeyT> Val) {<br class="">
BucketT *TheBucket;<br class="">
if (LookupBucketFor(Val, TheBucket))<br class="">
- return makeIterator(TheBucket, getBucketsEnd(), *this, true);<br class="">
+ return makeIterator(TheBucket,<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets()<br class="">
+ : getBucketsEnd(),<br class="">
+ *this, true);<br class="">
return end();<br class="">
}<br class="">
const_iterator find(const_arg_type_t<KeyT> Val) const {<br class="">
const BucketT *TheBucket;<br class="">
if (LookupBucketFor(Val, TheBucket))<br class="">
- return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);<br class="">
+ return makeConstIterator(TheBucket,<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets()<br class="">
+ : getBucketsEnd(),<br class="">
+ *this, true);<br class="">
return end();<br class="">
}<br class="">
<br class="">
@@ -168,14 +174,20 @@ class DenseMapBase : public DebugEpochBase {<br class="">
iterator find_as(const LookupKeyT &Val) {<br class="">
BucketT *TheBucket;<br class="">
if (LookupBucketFor(Val, TheBucket))<br class="">
- return makeIterator(TheBucket, getBucketsEnd(), *this, true);<br class="">
+ return makeIterator(TheBucket,<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets()<br class="">
+ : getBucketsEnd(),<br class="">
+ *this, true);<br class="">
return end();<br class="">
}<br class="">
template<class LookupKeyT><br class="">
const_iterator find_as(const LookupKeyT &Val) const {<br class="">
const BucketT *TheBucket;<br class="">
if (LookupBucketFor(Val, TheBucket))<br class="">
- return makeConstIterator(TheBucket, getBucketsEnd(), *this, true);<br class="">
+ return makeConstIterator(TheBucket,<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets()<br class="">
+ : getBucketsEnd(),<br class="">
+ *this, true);<br class="">
return end();<br class="">
}<br class="">
<br class="">
@@ -208,17 +220,17 @@ class DenseMapBase : public DebugEpochBase {<br class="">
template <typename... Ts><br class="">
std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&... Args) {<br class="">
BucketT *TheBucket;<br class="">
+ BucketT *EndBucket =<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br class="">
if (LookupBucketFor(Key, TheBucket))<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- false); // Already in map.<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ false); // Already in map.<br class="">
<br class="">
// Otherwise, insert the new element.<br class="">
TheBucket =<br class="">
InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- true);<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ true);<br class="">
}<br class="">
<br class="">
// Inserts key,value pair into the map if the key isn't already in the map.<br class="">
@@ -227,16 +239,16 @@ class DenseMapBase : public DebugEpochBase {<br class="">
template <typename... Ts><br class="">
std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&... Args) {<br class="">
BucketT *TheBucket;<br class="">
+ BucketT *EndBucket =<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br class="">
if (LookupBucketFor(Key, TheBucket))<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- false); // Already in map.<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ false); // Already in map.<br class="">
<br class="">
// Otherwise, insert the new element.<br class="">
TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- true);<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ true);<br class="">
}<br class="">
<br class="">
/// Alternate version of insert() which allows a <br class="">
diff erent, and possibly<br class="">
@@ -248,17 +260,17 @@ class DenseMapBase : public DebugEpochBase {<br class="">
std::pair<iterator, bool> insert_as(std::pair<KeyT, ValueT> &&KV,<br class="">
const LookupKeyT &Val) {<br class="">
BucketT *TheBucket;<br class="">
+ BucketT *EndBucket =<br class="">
+ shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();<br class="">
if (LookupBucketFor(Val, TheBucket))<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- false); // Already in map.<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ false); // Already in map.<br class="">
<br class="">
// Otherwise, insert the new element.<br class="">
TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),<br class="">
std::move(KV.second), Val);<br class="">
- return std::make_pair(<br class="">
- makeIterator(TheBucket, getBucketsEnd(), *this, true),<br class="">
- true);<br class="">
+ return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),<br class="">
+ true);<br class="">
}<br class="">
<br class="">
/// insert - Range insertion of pairs.<br class="">
@@ -1200,20 +1212,16 @@ class DenseMapIterator : DebugEpochBase::HandleBase {<br class="">
<br class="">
reference operator*() const {<br class="">
assert(isHandleInSync() && "invalid iterator access!");<br class="">
- if (shouldReverseIterate<KeyT>()) {<br class="">
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br class="">
- return Ptr[-1];<br class="">
- }<br class="">
assert(Ptr != End && "dereferencing end() iterator");<br class="">
+ if (shouldReverseIterate<KeyT>())<br class="">
+ return Ptr[-1];<br class="">
return *Ptr;<br class="">
}<br class="">
pointer operator->() const {<br class="">
assert(isHandleInSync() && "invalid iterator access!");<br class="">
- if (shouldReverseIterate<KeyT>()) {<br class="">
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br class="">
- return &(Ptr[-1]);<br class="">
- }<br class="">
assert(Ptr != End && "dereferencing end() iterator");<br class="">
+ if (shouldReverseIterate<KeyT>())<br class="">
+ return &(Ptr[-1]);<br class="">
return Ptr;<br class="">
}<br class="">
<br class="">
@@ -1234,13 +1242,12 @@ class DenseMapIterator : DebugEpochBase::HandleBase {<br class="">
<br class="">
inline DenseMapIterator& operator++() { // Preincrement<br class="">
assert(isHandleInSync() && "invalid iterator access!");<br class="">
+ assert(Ptr != End && "incrementing end() iterator");<br class="">
if (shouldReverseIterate<KeyT>()) {<br class="">
- assert(Ptr != &End[-1] && "dereferencing end() iterator");<br class="">
--Ptr;<br class="">
RetreatPastEmptyBuckets();<br class="">
return *this;<br class="">
}<br class="">
- assert(Ptr != End && "incrementing end() iterator");<br class="">
++Ptr;<br class="">
AdvancePastEmptyBuckets();<br class="">
return *this;<br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div>
</div></blockquote></div><br class=""></div></body></html>