<div dir="ltr">FYI, this also exposes some nondeterminism in the Clang Static Analyser, resulting in four test failures. Something will need to be done about those before this can be turned back on by default.<br><div class="gmail_extra"><br><div class="gmail_quote">On 13 December 2016 at 17:21, Quentin Colombet via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hi,<div><br></div><div>This seems to crash the bots.</div><div>E.g.,</div><div><a href="http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/31314/testReport/junit/LLVM/CodeGen_AMDGPU/ctlz_zero_undef_ll/" target="_blank">http://lab.llvm.org:8080/<wbr>green/job/clang-stage1-cmake-<wbr>RA-incremental_check/31314/<wbr>testReport/junit/LLVM/CodeGen_<wbr>AMDGPU/ctlz_zero_undef_ll/</a></div><div><br></div><div>Could you fix or revert?</div><div><br></div><div>Thanks,</div><div>-Quentin<div><div class="h5"><br><div><blockquote type="cite"><div>On Dec 13, 2016, at 4:15 PM, Mandeep Singh Grang via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="m_-2351234259198786128Apple-interchange-newline"><div><div>Author: mgrang<br>Date: Tue Dec 13 18:15:57 2016<br>New Revision: 289619<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=289619&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=289619&view=rev</a><br>Log:<br>[llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen<br><br>Summary:<br>Given a flag (-mllvm -reverse-iterate) this patch will enable iteration of SmallPtrSet in reverse order.<br>The idea is to compile the same source with and without this flag and expect the code to not change.<br>If there is a difference in codegen then it would mean that the codegen is sensitive to the iteration order of SmallPtrSet.<br>This is enabled only with LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS.<br><br>Reviewers: chandlerc, dexonsmith, mehdi_amini<br><br>Subscribers: mgorny, emaste, llvm-commits<br><br>Differential Revision: <a href="https://reviews.llvm.org/D26718" target="_blank">https://reviews.llvm.org/<wbr>D26718</a><br><br>Added:<br>    llvm/trunk/unittests/ADT/<wbr>ReverseIterationTest.cpp<br>Modified:<br>    llvm/trunk/include/llvm/<wbr>ADT/SmallPtrSet.h<br>    llvm/trunk/lib/Support/<wbr>CommandLine.cpp<br>    llvm/trunk/unittests/ADT/<wbr>CMakeLists.txt<br><br>Modified: llvm/trunk/include/llvm/ADT/<wbr>SmallPtrSet.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=289619&r1=289618&r2=289619&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/ADT/SmallPtrSet.h?rev=<wbr>289619&r1=289618&r2=289619&<wbr>view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/include/llvm/ADT/<wbr>SmallPtrSet.h (original)<br>+++ llvm/trunk/include/llvm/ADT/<wbr>SmallPtrSet.h Tue Dec 13 18:15:57 2016<br>@@ -15,6 +15,7 @@<br> #ifndef LLVM_ADT_SMALLPTRSET_H<br> #define LLVM_ADT_SMALLPTRSET_H<br><br>+#include "llvm/Config/abi-breaking.h"<br> #include "llvm/Support/Compiler.h"<br> #include "llvm/Support/<wbr>PointerLikeTypeTraits.h"<br> #include <cassert><br>@@ -25,6 +26,13 @@<br> #include <iterator><br> #include <utility><br><br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+namespace llvm {<br>+template <class T = void> struct ReverseIterate { static bool value; };<br>+template <class T> bool ReverseIterate<T>::value = true;<br>+}<br>+#endif<br>+<br> namespace llvm {<br><br> /// SmallPtrSetImplBase - This is the common code shared among all the<br>@@ -206,6 +214,12 @@ protected:<br> public:<br>   explicit SmallPtrSetIteratorImpl(const void *const *BP, const void*const *E)<br>     : Bucket(BP), End(E) {<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+    if (ReverseIterate<bool>::value) {<br>+      RetreatIfNotValid();<br>+      return;<br>+    }<br>+#endif<br>     AdvanceIfNotValid();<br>   }<br><br>@@ -227,6 +241,17 @@ protected:<br>             *Bucket == SmallPtrSetImplBase::<wbr>getTombstoneMarker()))<br>       ++Bucket;<br>   }<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+  void RetreatIfNotValid() {<br>+    --Bucket;<br>+    assert(Bucket <= End);<br>+    while (Bucket != End &&<br>+           (*Bucket == SmallPtrSetImplBase::<wbr>getEmptyMarker() ||<br>+            *Bucket == SmallPtrSetImplBase::<wbr>getTombstoneMarker())) {<br>+      --Bucket;<br>+    }<br>+  }<br>+#endif<br> };<br><br> /// SmallPtrSetIterator - This implements a const_iterator for SmallPtrSet.<br>@@ -252,13 +277,27 @@ public:<br>   }<br><br>   inline SmallPtrSetIterator& operator++() {          // Preincrement<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+    if (ReverseIterate<bool>::value) {<br>+      RetreatIfNotValid();<br>+      return *this;<br>+    }<br>+#endif<br>     ++Bucket;<br>     AdvanceIfNotValid();<br>     return *this;<br>   }<br><br>   SmallPtrSetIterator operator++(int) {        // Postincrement<br>-    SmallPtrSetIterator tmp = *this; ++*this; return tmp;<br>+    SmallPtrSetIterator tmp = *this;<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+    if (ReverseIterate<bool>::value) {<br>+      --*this;<br>+      return tmp;<br>+    }<br>+#endif<br>+    ++*this;<br>+    return tmp;<br>   }<br> };<br><br>@@ -343,9 +382,22 @@ public:<br>   }<br><br>   inline iterator begin() const {<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+    if (ReverseIterate<bool>::value)<br>+      return endPtr();<br>+#endif<br>     return iterator(CurArray, EndPointer());<br>   }<br>   inline iterator end() const {<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+    if (ReverseIterate<bool>::value)<br>+      return iterator(CurArray, CurArray);<br>+#endif<br>+    return endPtr();<br>+  }<br>+<br>+private:<br>+  inline iterator endPtr() const {<br>     const void *const *End = EndPointer();<br>     return iterator(End, End);<br>   }<br><br>Modified: llvm/trunk/lib/Support/<wbr>CommandLine.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=289619&r1=289618&r2=289619&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>Support/CommandLine.cpp?rev=<wbr>289619&r1=289618&r2=289619&<wbr>view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/lib/Support/<wbr>CommandLine.cpp (original)<br>+++ llvm/trunk/lib/Support/<wbr>CommandLine.cpp Tue Dec 13 18:15:57 2016<br>@@ -45,6 +45,17 @@ using namespace cl;<br><br> #define DEBUG_TYPE "commandline"<br><br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+namespace llvm {<br>+// If LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS is set the flag -mllvm -reverse-iterate<br>+// can be used to toggle forward/reverse iteration of unordered containers.<br>+// This will help uncover differences in codegen caused due to undefined<br>+// iteration order.<br>+static cl::opt<bool, true> ReverseIteration("reverse-<wbr>iterate",<br>+  cl::location(ReverseIterate<<wbr>bool>::value), cl::init(true));<br>+}<br>+#endif<br>+<br> //===-------------------------<wbr>------------------------------<wbr>---------------===//<br> // Template instantiations and anchors.<br> //<br><br>Modified: llvm/trunk/unittests/ADT/<wbr>CMakeLists.txt<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=289619&r1=289618&r2=289619&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/unittests/<wbr>ADT/CMakeLists.txt?rev=289619&<wbr>r1=289618&r2=289619&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/unittests/ADT/<wbr>CMakeLists.txt (original)<br>+++ llvm/trunk/unittests/ADT/<wbr>CMakeLists.txt Tue Dec 13 18:15:57 2016<br>@@ -41,6 +41,7 @@ set(ADTSources<br>   PostOrderIteratorTest.cpp<br>   PriorityWorklistTest.cpp<br>   RangeAdapterTest.cpp<br>+  ReverseIterationTest.cpp<br>   SCCIteratorTest.cpp<br>   STLExtrasTest.cpp<br>   ScopeExitTest.cpp<br><br>Added: llvm/trunk/unittests/ADT/<wbr>ReverseIterationTest.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ReverseIterationTest.cpp?rev=289619&view=auto" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/unittests/<wbr>ADT/ReverseIterationTest.cpp?<wbr>rev=289619&view=auto</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/unittests/ADT/<wbr>ReverseIterationTest.cpp (added)<br>+++ llvm/trunk/unittests/ADT/<wbr>ReverseIterationTest.cpp Tue Dec 13 18:15:57 2016<br>@@ -0,0 +1,39 @@<br>+//===- llvm/unittest/ADT/<wbr>ReverseIterationTest.cpp ------------------------------<wbr>===//<br>+//<br>+//                     The LLVM Compiler Infrastructure<br>+//<br>+// This file is distributed under the University of Illinois Open Source<br>+// License. See LICENSE.TXT for details.<br>+//<br>+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>+//<br>+// ReverseIteration unit tests.<br>+//<br>+//===------------------------<wbr>------------------------------<wbr>----------------===//<br>+<br>+#include "gtest/gtest.h"<br>+#include "llvm/ADT/SmallPtrSet.h"<br>+<br>+#if LLVM_ENABLE_ABI_BREAKING_<wbr>CHECKS<br>+using namespace llvm;<br>+<br>+TEST(ReverseIterationTest, SmallPtrSetTest) {<br>+<br>+  SmallPtrSet<void*, 4> Set;<br>+  void *Ptrs[] = { (void*)0x1, (void*)0x2, (void*)0x3, (void*)0x4 };<br>+  void *ReversePtrs[] = { (void*)0x4, (void*)0x3, (void*)0x2, (void*)0x1 };<br>+<br>+  for (auto *Ptr: Ptrs)<br>+    Set.insert(Ptr);<br>+<br>+  // Check forward iteration.<br>+  ReverseIterate<bool>::value = false;<br>+  for (const auto &Tuple : zip(Set, Ptrs))<br>+    ASSERT_EQ(std::get<0>(<wbr>Tuple), std::get<1>(Tuple));<br>+<br>+  // Check reverse iteration.<br>+  ReverseIterate<bool>::value = true;<br>+  for (const auto &Tuple : zip(Set, ReversePtrs))<br>+    ASSERT_EQ(std::get<0>(<wbr>Tuple), std::get<1>(Tuple));<br>+}<br>+#endif<br><br><br>______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br></div></div></blockquote></div><br></div></div></div></div><br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>