[PATCH] D112175: [NFC] Add llvm::StaticVector ADT

[James Player via Phabricator via llvm-commits]

jplayer-nv added a comment.

>   [ RUN      ] StaticVsSmall/0.TimedStaticVector
>   [       OK ] StaticVsSmall/0.TimedStaticVector (371 ms)
>   [ RUN      ] StaticVsSmall/0.TimedSmallVector
>   [       OK ] StaticVsSmall/0.TimedSmallVector (596 ms)
>
> Which demonstrates that simple insertions are significantly faster (almost 38%) with the StaticVector compared to a SmallVector which stays within its small buffer limit.

Let me amend this a bit... it looks like the reason `SmallVector::push_back` is slower in this case is because of insufficient specialization.  Here's the trivially copyable version:

  void push_back(ValueParamT Elt) {
    const T *EltPtr = reserveForParamAndGetAddress(Elt);
    memcpy(reinterpret_cast<void *>(this->end()), EltPtr, sizeof(T));
    this->set_size(this->size() + 1);
  }

When `Elt` is passed by value, `reserveForParamAndGetAddress()` is unnecessary since the parameter will not alias with the `SmallVector`'s buffer.  This inhibits host compiler optimization.  If we were to SFINAE split this function, we can match my `StaticVector::push_back` perf.  The following `SmallVector` change works for me locally:

    template <typename RetT = std::enable_if_t<TakesParamByValue>>
    RetT push_back_impl(value_type Elt) {
      if (LLVM_UNLIKELY(this->size() >= this->capacity()))
        this->grow();
      memcpy(reinterpret_cast<void *>(this->end()), std::addressof(Elt),
             sizeof(T));
      this->set_size(this->size() + 1);
    }
  
    template <typename RetT = std::enable_if_t<!TakesParamByValue>>
    RetT push_back_impl(const_reference Elt) {
      const T *EltPtr = reserveForParamAndGetAddress(Elt);
      memcpy(reinterpret_cast<void *>(this->end()), EltPtr, sizeof(T));
      this->set_size(this->size() + 1);
    }
  
  public:
    void push_back(ValueParamT Elt) { push_back_impl(Elt); }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112175/new/

https://reviews.llvm.org/D112175