[llvm-commits] [llvm] r149851 - /llvm/trunk/include/llvm/ADT/SmallVector.h

[Bob Wilson]

On Feb 5, 2012, at 2:48 PM, Benjamin Kramer <benny.kra at googlemail.com> wrote:

> Author: d0k
> Date: Sun Feb  5 16:48:31 2012
> New Revision: 149851
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=149851&view=rev
> Log:
> SmallVector's construct_range is the same thing as std::uninitialized_fill, no need to reinvent it.
> 
> Modified:
>    llvm/trunk/include/llvm/ADT/SmallVector.h

We're seeing a very small but repeatable compile regression from this change.  For example, compiling MultiSource/Benchmarks/MiBench/consumer-typeset we measured an increase from 2.766 to 2.785 seconds (user time).  The std deviation on those times is high so it may be a little bit hard to see the change.

Daniel did some quick investigation and it seemed like there was a difference in loop unrolling between the two versions.  He looked at a simple function like:

#include "llvm/ADT/SmallVector.h"
using namespace llvm;

void f0(SmallVector<int, 32> a) {
  a.assign(20, 0);
}

When compiled with the old version (using construct_range), the IR does not get unrolled. When compiled with std::uninitialized_fill then the IR does get unrolled.  Any chance you'd be up for investigating that?  If not, how would you feel about temporarily reverting the patch until someone has a chance to fix up the optimizer to handle this better?


> 
> Modified: llvm/trunk/include/llvm/ADT/SmallVector.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallVector.h?rev=149851&r1=149850&r2=149851&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/SmallVector.h (original)
> +++ llvm/trunk/include/llvm/ADT/SmallVector.h Sun Feb  5 16:48:31 2012
> @@ -299,7 +299,7 @@
>     } else if (N > this->size()) {
>       if (this->capacity() < N)
>         this->grow(N);
> -      this->construct_range(this->end(), this->begin()+N, T());
> +      std::uninitialized_fill(this->end(), this->begin()+N, T());
>       this->setEnd(this->begin()+N);
>     }
>   }
> @@ -311,7 +311,7 @@
>     } else if (N > this->size()) {
>       if (this->capacity() < N)
>         this->grow(N);
> -      construct_range(this->end(), this->begin()+N, NV);
> +      std::uninitialized_fill(this->end(), this->begin()+N, NV);
>       this->setEnd(this->begin()+N);
>     }
>   }
> @@ -378,7 +378,7 @@
>     if (this->capacity() < NumElts)
>       this->grow(NumElts);
>     this->setEnd(this->begin()+NumElts);
> -    construct_range(this->begin(), this->end(), Elt);
> +    std::uninitialized_fill(this->begin(), this->end(), Elt);
>   }
> 
>   iterator erase(iterator I) {
> @@ -556,12 +556,6 @@
>     assert(N <= this->capacity());
>     this->setEnd(this->begin() + N);
>   }
> -
> -private:
> -  static void construct_range(T *S, T *E, const T &Elt) {
> -    for (; S != E; ++S)
> -      new (S) T(Elt);
> -  }
> };
> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits