[llvm] r289619 - [llvm] Iterate SmallPtrSet in reverse order to uncover non-determinism in codegen
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 7 18:38:11 PST 2017
No problem!
> On Mar 7, 2017, at 18:31, Daniel Berlin <dberlin at dberlin.org> wrote:
>
> and also, of course, thanks for fixing it.
> Much appreciated ;)
>
>
> On Tue, Mar 7, 2017 at 6:29 PM, Daniel Berlin <dberlin at dberlin.org <mailto:dberlin at dberlin.org>> wrote:
> Thank you, couldn't find the commit thread till you replied :)
>
>
> On Tue, Mar 7, 2017 at 2:12 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> r297234 should fix it.
>
>> On 2017-Mar-07, at 11:25, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
>>
>> [DannyB replied to the old phab review, but let's move the discussion to the commit.]
>>
>> My guess is that find() should be updated to return end().
>>
>>> On 2017-Mar-07, at 10:57, Daniel Berlin <dberlin at dberlin.org <mailto:dberlin at dberlin.org>> wrote:
>>>
>>> (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)
>>>
>>>
>>> On Tue, Mar 7, 2017 at 10:43 AM, Daniel Berlin <dberlin at dberlin.org <mailto:dberlin at dberlin.org>> wrote:
>>> So, this change breaks smallptrset's find function when it is on.
>>>
>>> Trivial test case:
>>> SmallPtrSet<int *, 8> IntSet;
>>> int A;
>>> int *B = &A;
>>> EXPECT_FALSE(IntSet.find(B) == IntSet.end());
>>>
>>> Because you've made the end iterator point somewhere else, this will no longer work.
>>>
>>> Please fix.
>>>
>>
>>
>>> On 2016-Dec-13, at 16:15, Mandeep Singh Grang via llvm-commits <llvm-commits at lists.llvm.org <mailto: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 <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 <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 <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 <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 <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 <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 <mailto:llvm-commits at lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/20170307/26bbcd59/attachment.html>
More information about the llvm-commits
mailing list