[llvm-commits] [llvm] r165065 - in /llvm/trunk: include/llvm/ADT/SetVector.h lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/basictest.ll
Chandler Carruth
chandlerc at gmail.com
Tue Oct 2 17:05:25 PDT 2012
On Tue, Oct 2, 2012 at 4:26 PM, Sean Silva <silvas at purdue.edu> wrote:
> > + template <typename UnaryPredicate>
> > + bool remove_if(UnaryPredicate P) {
> > + typename vector_type::iterator B = std::remove_if(vector_.begin(),
> > + vector_.end(), P),
> > + E = vector_.end();
> > + if (B == E)
> > + return false;
> > + for (typename vector_type::iterator I = B; I != E; ++I)
> > + set_.erase(*I);
> > + vector_.erase(B, E);
> > + return true;
> > + }
> > +
>
> I don't think that remove_if is allowed to be used like this.
>
> From 25.3.8 [alg.remove]:
> Note: each element in the range [ret,last), where ret is the returned
> value, has a valid but unspeciļ¬ed state, because the algorithms can
> eliminate elements by swapping with or moving from elements that were
> originally in that range.
>
>
> I think you can use stable_partition() to do what you want here, but
> that's pretty expensive (comparatively; it works by recursively
> rotating in specific ways; n*log n but also the constant factor
> compared with just a linear scan of an array). The simplest
> alternative is probably to just open code remove_if(), and do the
> deletion inside the open-coded remove_if() loop as you "delete"
> elements (i.e. overwrite them).
>
Thanks, this is a great point.
No need to write out remove_if -- std::partition() should be just as
efficient and provides the guarantee we need.
>
> --Sean Silva
>
> On Tue, Oct 2, 2012 at 6:46 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
> > Author: chandlerc
> > Date: Tue Oct 2 17:46:45 2012
> > New Revision: 165065
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=165065&view=rev
> > Log:
> > Teach the new SROA to handle cases where an alloca that has already been
> > scheduled for processing on the worklist eventually gets deleted while
> > we are processing another alloca, fixing the original test case in
> > PR13990.
> >
> > To facilitate this, add a remove_if helper to the SetVector abstraction.
> > It's not easy to use the standard abstractions for this because of the
> > specifics of SetVectors types and implementation.
> >
> > Finally, a nice small test case is included. Thanks to Benjamin for the
> > fantastic reduced test case here! All I had to do was delete some empty
> > basic blocks!
> >
> > Modified:
> > llvm/trunk/include/llvm/ADT/SetVector.h
> > llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> > llvm/trunk/test/Transforms/SROA/basictest.ll
> >
> > Modified: llvm/trunk/include/llvm/ADT/SetVector.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SetVector.h?rev=165065&r1=165064&r2=165065&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/ADT/SetVector.h (original)
> > +++ llvm/trunk/include/llvm/ADT/SetVector.h Tue Oct 2 17:46:45 2012
> > @@ -126,6 +126,32 @@
> > return false;
> > }
> >
> > + /// \brief Remove items from the set vector based on a predicate
> function.
> > + ///
> > + /// This is intended to be equivalent to the following code, if we
> could
> > + /// write it:
> > + ///
> > + /// \code
> > + /// V.erase(std::remove_if(V.begin(), V.end(), P), V.end());
> > + /// \endcode
> > + ///
> > + /// However, SetVector doesn't expose non-const iterators, making any
> > + /// algorithm like remove_if impossible to use.
> > + ///
> > + /// \returns true if any element is removed.
> > + template <typename UnaryPredicate>
> > + bool remove_if(UnaryPredicate P) {
> > + typename vector_type::iterator B = std::remove_if(vector_.begin(),
> > + vector_.end(), P),
> > + E = vector_.end();
> > + if (B == E)
> > + return false;
> > + for (typename vector_type::iterator I = B; I != E; ++I)
> > + set_.erase(*I);
> > + vector_.erase(B, E);
> > + return true;
> > + }
> > +
> >
> > /// \brief Count the number of elements of a given key in the
> SetVector.
> > /// \returns 0 if the element is not in the SetVector, 1 if it is.
> >
> > Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=165065&r1=165064&r2=165065&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Oct 2 17:46:45 2012
> > @@ -3302,7 +3302,11 @@
> > while (!Worklist.empty()) {
> > Changed |= runOnAlloca(*Worklist.pop_back_val());
> > deleteDeadInstructions(DeletedAllocas);
> > +
> > + // Remove the deleted allocas from various lists so that we don't
> try to
> > + // continue processing them.
> > if (!DeletedAllocas.empty()) {
> > + Worklist.remove_if(IsAllocaInSet(DeletedAllocas));
> > PromotableAllocas.erase(std::remove_if(PromotableAllocas.begin(),
> > PromotableAllocas.end(),
> >
> IsAllocaInSet(DeletedAllocas)),
> >
> > Modified: llvm/trunk/test/Transforms/SROA/basictest.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/basictest.ll?rev=165065&r1=165064&r2=165065&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/SROA/basictest.ll (original)
> > +++ llvm/trunk/test/Transforms/SROA/basictest.ll Tue Oct 2 17:46:45 2012
> > @@ -897,3 +897,32 @@
> > %tmp2 = load i8* %gep
> > ret void
> > }
> > +
> > +define void @PR13990() {
> > +; Ensure we can handle cases where processing one alloca causes the
> other
> > +; alloca to become dead and get deleted. This might crash or fail under
> > +; Valgrind if we regress.
> > +; CHECK: @PR13990
> > +; CHECK-NOT: alloca
> > +; CHECK: unreachable
> > +; CHECK: unreachable
> > +
> > +entry:
> > + %tmp1 = alloca i8*
> > + %tmp2 = alloca i8*
> > + br i1 undef, label %bb1, label %bb2
> > +
> > +bb1:
> > + store i8* undef, i8** %tmp2
> > + br i1 undef, label %bb2, label %bb3
> > +
> > +bb2:
> > + %tmp50 = select i1 undef, i8** %tmp2, i8** %tmp1
> > + br i1 undef, label %bb3, label %bb4
> > +
> > +bb3:
> > + unreachable
> > +
> > +bb4:
> > + unreachable
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121002/7fdb2896/attachment.html>
More information about the llvm-commits
mailing list