<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">r297234 should fix it.<div class=""><br class=""><div class=""><div><blockquote type="cite" class=""><div class="">On 2017-Mar-07, at 11:25, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">[DannyB replied to the old phab review, but let's move the discussion to the commit.]<div class=""><br class=""></div><div class="">My guess is that find() should be updated to return end().</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div class="">On 2017-Mar-07, at 10:57, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" class="">dberlin@dberlin.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">(and FWIW, the reason it needs fixing is it makes it impossible to use find in unit tests, see the FIXME i'm about to commit to smallptrsettest)<div class=""><br class=""></div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Mar 7, 2017 at 10:43 AM, Daniel Berlin <span dir="ltr" class=""><<a href="mailto:dberlin@dberlin.org" target="_blank" class="">dberlin@dberlin.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class="">So, this change breaks smallptrset's find function when it is on.<div class=""><br class=""></div><div class="">Trivial test case:<br class=""><div class=""> SmallPtrSet<int *, 8> IntSet;</div><div class=""> int A;</div><div class=""> int *B = &A;</div></div><div class=""><div class=""> EXPECT_FALSE(IntSet.find(B) == IntSet.end());</div></div><div class=""><br class=""></div><div class="">Because you've made the end iterator point somewhere else, this will no longer work.</div><div class=""><br class=""></div><div class="">Please fix.</div><div class=""><br class=""></div></div><div class="HOEnZb"><div class="h5"></div></div></blockquote></div></div></div></blockquote></div></div></blockquote><div class=""><br class=""></div><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On 2016-Dec-13, at 16:15, Mandeep Singh Grang via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: mgrang<br class="">Date: Tue Dec 13 18:15:57 2016<br class="">New Revision: 289619<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=289619&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=289619&view=rev</a><br class="">Log:<br class="">[llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen<br class=""><br class="">Summary:<br class="">Given a flag (-mllvm -reverse-iterate) this patch will enable iteration of SmallPtrSet in reverse order.<br class="">The idea is to compile the same source with and without this flag and expect the code to not change.<br class="">If there is a difference in codegen then it would mean that the codegen is sensitive to the iteration order of SmallPtrSet.<br class="">This is enabled only with LLVM_ENABLE_ABI_BREAKING_CHECKS.<br class=""><br class="">Reviewers: chandlerc, dexonsmith, mehdi_amini<br class=""><br class="">Subscribers: mgorny, emaste, llvm-commits<br class=""><br class="">Differential Revision: <a href="https://reviews.llvm.org/D26718" class="">https://reviews.llvm.org/D26718</a><br class=""><br class="">Added:<br class=""> llvm/trunk/unittests/ADT/ReverseIterationTest.cpp<br class="">Modified:<br class=""> llvm/trunk/include/llvm/ADT/SmallPtrSet.h<br class=""> llvm/trunk/lib/Support/CommandLine.cpp<br class=""> llvm/trunk/unittests/ADT/CMakeLists.txt<br class=""><br class="">Modified: llvm/trunk/include/llvm/ADT/SmallPtrSet.h<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=289619&r1=289618&r2=289619&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=289619&r1=289618&r2=289619&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/ADT/SmallPtrSet.h (original)<br class="">+++ llvm/trunk/include/llvm/ADT/SmallPtrSet.h Tue Dec 13 18:15:57 2016<br class="">@@ -15,6 +15,7 @@<br class=""> #ifndef LLVM_ADT_SMALLPTRSET_H<br class=""> #define LLVM_ADT_SMALLPTRSET_H<br class=""><br class="">+#include "llvm/Config/abi-breaking.h"<br class=""> #include "llvm/Support/Compiler.h"<br class=""> #include "llvm/Support/PointerLikeTypeTraits.h"<br class=""> #include <cassert><br class="">@@ -25,6 +26,13 @@<br class=""> #include <iterator><br class=""> #include <utility><br class=""><br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+namespace llvm {<br class="">+template <class T = void> struct ReverseIterate { static bool value; };<br class="">+template <class T> bool ReverseIterate<T>::value = true;<br class="">+}<br class="">+#endif<br class="">+<br class=""> namespace llvm {<br class=""><br class=""> /// SmallPtrSetImplBase - This is the common code shared among all the<br class="">@@ -206,6 +214,12 @@ protected:<br class=""> public:<br class=""> explicit SmallPtrSetIteratorImpl(const void *const *BP, const void*const *E)<br class=""> : Bucket(BP), End(E) {<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ if (ReverseIterate<bool>::value) {<br class="">+ RetreatIfNotValid();<br class="">+ return;<br class="">+ }<br class="">+#endif<br class=""> AdvanceIfNotValid();<br class=""> }<br class=""><br class="">@@ -227,6 +241,17 @@ protected:<br class=""> *Bucket == SmallPtrSetImplBase::getTombstoneMarker()))<br class=""> ++Bucket;<br class=""> }<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ void RetreatIfNotValid() {<br class="">+ --Bucket;<br class="">+ assert(Bucket <= End);<br class="">+ while (Bucket != End &&<br class="">+ (*Bucket == SmallPtrSetImplBase::getEmptyMarker() ||<br class="">+ *Bucket == SmallPtrSetImplBase::getTombstoneMarker())) {<br class="">+ --Bucket;<br class="">+ }<br class="">+ }<br class="">+#endif<br class=""> };<br class=""><br class=""> /// SmallPtrSetIterator - This implements a const_iterator for SmallPtrSet.<br class="">@@ -252,13 +277,27 @@ public:<br class=""> }<br class=""><br class=""> inline SmallPtrSetIterator& operator++() { // Preincrement<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ if (ReverseIterate<bool>::value) {<br class="">+ RetreatIfNotValid();<br class="">+ return *this;<br class="">+ }<br class="">+#endif<br class=""> ++Bucket;<br class=""> AdvanceIfNotValid();<br class=""> return *this;<br class=""> }<br class=""><br class=""> SmallPtrSetIterator operator++(int) { // Postincrement<br class="">- SmallPtrSetIterator tmp = *this; ++*this; return tmp;<br class="">+ SmallPtrSetIterator tmp = *this;<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ if (ReverseIterate<bool>::value) {<br class="">+ --*this;<br class="">+ return tmp;<br class="">+ }<br class="">+#endif<br class="">+ ++*this;<br class="">+ return tmp;<br class=""> }<br class=""> };<br class=""><br class="">@@ -343,9 +382,22 @@ public:<br class=""> }<br class=""><br class=""> inline iterator begin() const {<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ if (ReverseIterate<bool>::value)<br class="">+ return endPtr();<br class="">+#endif<br class=""> return iterator(CurArray, EndPointer());<br class=""> }<br class=""> inline iterator end() const {<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+ if (ReverseIterate<bool>::value)<br class="">+ return iterator(CurArray, CurArray);<br class="">+#endif<br class="">+ return endPtr();<br class="">+ }<br class="">+<br class="">+private:<br class="">+ inline iterator endPtr() const {<br class=""> const void *const *End = EndPointer();<br class=""> return iterator(End, End);<br class=""> }<br class=""><br class="">Modified: llvm/trunk/lib/Support/CommandLine.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=289619&r1=289618&r2=289619&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=289619&r1=289618&r2=289619&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Support/CommandLine.cpp (original)<br class="">+++ llvm/trunk/lib/Support/CommandLine.cpp Tue Dec 13 18:15:57 2016<br class="">@@ -45,6 +45,17 @@ using namespace cl;<br class=""><br class=""> #define DEBUG_TYPE "commandline"<br class=""><br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+namespace llvm {<br class="">+// If LLVM_ENABLE_ABI_BREAKING_CHECKS is set the flag -mllvm -reverse-iterate<br class="">+// can be used to toggle forward/reverse iteration of unordered containers.<br class="">+// This will help uncover differences in codegen caused due to undefined<br class="">+// iteration order.<br class="">+static cl::opt<bool, true> ReverseIteration("reverse-iterate",<br class="">+ cl::location(ReverseIterate<bool>::value), cl::init(true));<br class="">+}<br class="">+#endif<br class="">+<br class=""> //===----------------------------------------------------------------------===//<br class=""> // Template instantiations and anchors.<br class=""> //<br class=""><br class="">Modified: llvm/trunk/unittests/ADT/CMakeLists.txt<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=289619&r1=289618&r2=289619&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=289619&r1=289618&r2=289619&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/unittests/ADT/CMakeLists.txt (original)<br class="">+++ llvm/trunk/unittests/ADT/CMakeLists.txt Tue Dec 13 18:15:57 2016<br class="">@@ -41,6 +41,7 @@ set(ADTSources<br class=""> PostOrderIteratorTest.cpp<br class=""> PriorityWorklistTest.cpp<br class=""> RangeAdapterTest.cpp<br class="">+ ReverseIterationTest.cpp<br class=""> SCCIteratorTest.cpp<br class=""> STLExtrasTest.cpp<br class=""> ScopeExitTest.cpp<br class=""><br class="">Added: llvm/trunk/unittests/ADT/ReverseIterationTest.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ReverseIterationTest.cpp?rev=289619&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ReverseIterationTest.cpp?rev=289619&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/unittests/ADT/ReverseIterationTest.cpp (added)<br class="">+++ llvm/trunk/unittests/ADT/ReverseIterationTest.cpp Tue Dec 13 18:15:57 2016<br class="">@@ -0,0 +1,39 @@<br class="">+//===- llvm/unittest/ADT/ReverseIterationTest.cpp ------------------------------===//<br class="">+//<br class="">+// The LLVM Compiler Infrastructure<br class="">+//<br class="">+// This file is distributed under the University of Illinois Open Source<br class="">+// License. See LICENSE.TXT for details.<br class="">+//<br class="">+//===----------------------------------------------------------------------===//<br class="">+//<br class="">+// ReverseIteration unit tests.<br class="">+//<br class="">+//===----------------------------------------------------------------------===//<br class="">+<br class="">+#include "gtest/gtest.h"<br class="">+#include "llvm/ADT/SmallPtrSet.h"<br class="">+<br class="">+#if LLVM_ENABLE_ABI_BREAKING_CHECKS<br class="">+using namespace llvm;<br class="">+<br class="">+TEST(ReverseIterationTest, SmallPtrSetTest) {<br class="">+<br class="">+ SmallPtrSet<void*, 4> Set;<br class="">+ void *Ptrs[] = { (void*)0x1, (void*)0x2, (void*)0x3, (void*)0x4 };<br class="">+ void *ReversePtrs[] = { (void*)0x4, (void*)0x3, (void*)0x2, (void*)0x1 };<br class="">+<br class="">+ for (auto *Ptr: Ptrs)<br class="">+ Set.insert(Ptr);<br class="">+<br class="">+ // Check forward iteration.<br class="">+ ReverseIterate<bool>::value = false;<br class="">+ for (const auto &Tuple : zip(Set, Ptrs))<br class="">+ ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));<br class="">+<br class="">+ // Check reverse iteration.<br class="">+ ReverseIterate<bool>::value = true;<br class="">+ for (const auto &Tuple : zip(Set, ReversePtrs))<br class="">+ ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));<br class="">+}<br class="">+#endif<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class=""></div></div></blockquote></div><br class=""></div></div></div></div></blockquote></div><br class=""></div></div></body></html>