[llvm] r289619 - [llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 17:21:45 PST 2016


Hi,

This seems to crash the bots.
E.g.,
http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/31314/testReport/junit/LLVM/CodeGen_AMDGPU/ctlz_zero_undef_ll/ <http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/31314/testReport/junit/LLVM/CodeGen_AMDGPU/ctlz_zero_undef_ll/>

Could you fix or revert?

Thanks,
-Quentin
> On Dec 13, 2016, at 4:15 PM, Mandeep Singh Grang via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Author: mgrang
> Date: Tue Dec 13 18:15:57 2016
> New Revision: 289619
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=289619&view=rev
> Log:
> [llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen
> 
> Summary:
> Given a flag (-mllvm -reverse-iterate) this patch will enable iteration of SmallPtrSet in reverse order.
> The idea is to compile the same source with and without this flag and expect the code to not change.
> If there is a difference in codegen then it would mean that the codegen is sensitive to the iteration order of SmallPtrSet.
> This is enabled only with LLVM_ENABLE_ABI_BREAKING_CHECKS.
> 
> Reviewers: chandlerc, dexonsmith, mehdi_amini
> 
> Subscribers: mgorny, emaste, llvm-commits
> 
> Differential Revision: https://reviews.llvm.org/D26718
> 
> Added:
>    llvm/trunk/unittests/ADT/ReverseIterationTest.cpp
> Modified:
>    llvm/trunk/include/llvm/ADT/SmallPtrSet.h
>    llvm/trunk/lib/Support/CommandLine.cpp
>    llvm/trunk/unittests/ADT/CMakeLists.txt
> 
> Modified: llvm/trunk/include/llvm/ADT/SmallPtrSet.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallPtrSet.h?rev=289619&r1=289618&r2=289619&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallPtrSet.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallPtrSet.h Tue Dec 13 18:15:57 2016
> @@ -15,6 +15,7 @@
> #ifndef LLVM_ADT_SMALLPTRSET_H
> #define LLVM_ADT_SMALLPTRSET_H
> 
> +#include "llvm/Config/abi-breaking.h"
> #include "llvm/Support/Compiler.h"
> #include "llvm/Support/PointerLikeTypeTraits.h"
> #include <cassert>
> @@ -25,6 +26,13 @@
> #include <iterator>
> #include <utility>
> 
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +namespace llvm {
> +template <class T = void> struct ReverseIterate { static bool value; };
> +template <class T> bool ReverseIterate<T>::value = true;
> +}
> +#endif
> +
> namespace llvm {
> 
> /// SmallPtrSetImplBase - This is the common code shared among all the
> @@ -206,6 +214,12 @@ protected:
> public:
>   explicit SmallPtrSetIteratorImpl(const void *const *BP, const void*const *E)
>     : Bucket(BP), End(E) {
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +    if (ReverseIterate<bool>::value) {
> +      RetreatIfNotValid();
> +      return;
> +    }
> +#endif
>     AdvanceIfNotValid();
>   }
> 
> @@ -227,6 +241,17 @@ protected:
>             *Bucket == SmallPtrSetImplBase::getTombstoneMarker()))
>       ++Bucket;
>   }
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +  void RetreatIfNotValid() {
> +    --Bucket;
> +    assert(Bucket <= End);
> +    while (Bucket != End &&
> +           (*Bucket == SmallPtrSetImplBase::getEmptyMarker() ||
> +            *Bucket == SmallPtrSetImplBase::getTombstoneMarker())) {
> +      --Bucket;
> +    }
> +  }
> +#endif
> };
> 
> /// SmallPtrSetIterator - This implements a const_iterator for SmallPtrSet.
> @@ -252,13 +277,27 @@ public:
>   }
> 
>   inline SmallPtrSetIterator& operator++() {          // Preincrement
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +    if (ReverseIterate<bool>::value) {
> +      RetreatIfNotValid();
> +      return *this;
> +    }
> +#endif
>     ++Bucket;
>     AdvanceIfNotValid();
>     return *this;
>   }
> 
>   SmallPtrSetIterator operator++(int) {        // Postincrement
> -    SmallPtrSetIterator tmp = *this; ++*this; return tmp;
> +    SmallPtrSetIterator tmp = *this;
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +    if (ReverseIterate<bool>::value) {
> +      --*this;
> +      return tmp;
> +    }
> +#endif
> +    ++*this;
> +    return tmp;
>   }
> };
> 
> @@ -343,9 +382,22 @@ public:
>   }
> 
>   inline iterator begin() const {
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +    if (ReverseIterate<bool>::value)
> +      return endPtr();
> +#endif
>     return iterator(CurArray, EndPointer());
>   }
>   inline iterator end() const {
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +    if (ReverseIterate<bool>::value)
> +      return iterator(CurArray, CurArray);
> +#endif
> +    return endPtr();
> +  }
> +
> +private:
> +  inline iterator endPtr() const {
>     const void *const *End = EndPointer();
>     return iterator(End, End);
>   }
> 
> Modified: llvm/trunk/lib/Support/CommandLine.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=289619&r1=289618&r2=289619&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
> +++ llvm/trunk/lib/Support/CommandLine.cpp Tue Dec 13 18:15:57 2016
> @@ -45,6 +45,17 @@ using namespace cl;
> 
> #define DEBUG_TYPE "commandline"
> 
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +namespace llvm {
> +// If LLVM_ENABLE_ABI_BREAKING_CHECKS is set the flag -mllvm -reverse-iterate
> +// can be used to toggle forward/reverse iteration of unordered containers.
> +// This will help uncover differences in codegen caused due to undefined
> +// iteration order.
> +static cl::opt<bool, true> ReverseIteration("reverse-iterate",
> +  cl::location(ReverseIterate<bool>::value), cl::init(true));
> +}
> +#endif
> +
> //===----------------------------------------------------------------------===//
> // Template instantiations and anchors.
> //
> 
> Modified: llvm/trunk/unittests/ADT/CMakeLists.txt
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/CMakeLists.txt?rev=289619&r1=289618&r2=289619&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/CMakeLists.txt (original)
> +++ llvm/trunk/unittests/ADT/CMakeLists.txt Tue Dec 13 18:15:57 2016
> @@ -41,6 +41,7 @@ set(ADTSources
>   PostOrderIteratorTest.cpp
>   PriorityWorklistTest.cpp
>   RangeAdapterTest.cpp
> +  ReverseIterationTest.cpp
>   SCCIteratorTest.cpp
>   STLExtrasTest.cpp
>   ScopeExitTest.cpp
> 
> Added: llvm/trunk/unittests/ADT/ReverseIterationTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ReverseIterationTest.cpp?rev=289619&view=auto
> ==============================================================================
> --- llvm/trunk/unittests/ADT/ReverseIterationTest.cpp (added)
> +++ llvm/trunk/unittests/ADT/ReverseIterationTest.cpp Tue Dec 13 18:15:57 2016
> @@ -0,0 +1,39 @@
> +//===- llvm/unittest/ADT/ReverseIterationTest.cpp ------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// ReverseIteration unit tests.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "gtest/gtest.h"
> +#include "llvm/ADT/SmallPtrSet.h"
> +
> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
> +using namespace llvm;
> +
> +TEST(ReverseIterationTest, SmallPtrSetTest) {
> +
> +  SmallPtrSet<void*, 4> Set;
> +  void *Ptrs[] = { (void*)0x1, (void*)0x2, (void*)0x3, (void*)0x4 };
> +  void *ReversePtrs[] = { (void*)0x4, (void*)0x3, (void*)0x2, (void*)0x1 };
> +
> +  for (auto *Ptr: Ptrs)
> +    Set.insert(Ptr);
> +
> +  // Check forward iteration.
> +  ReverseIterate<bool>::value = false;
> +  for (const auto &Tuple : zip(Set, Ptrs))
> +    ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));
> +
> +  // Check reverse iteration.
> +  ReverseIterate<bool>::value = true;
> +  for (const auto &Tuple : zip(Set, ReversePtrs))
> +    ASSERT_EQ(std::get<0>(Tuple), std::get<1>(Tuple));
> +}
> +#endif
> 
> 
> _______________________________________________
> 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/20161213/e2cf582f/attachment.html>


More information about the llvm-commits mailing list