[llvm-commits] [llvm] r165065 - in /llvm/trunk: include/llvm/ADT/SetVector.h lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/basictest.ll
Sean Silva
silvas at purdue.edu
Tue Oct 2 16:26:05 PDT 2012
> + 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).
--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
More information about the llvm-commits
mailing list