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

[James Player via Phabricator via llvm-commits]

jplayer-nv added a comment.

In D112175#3482069 <https://reviews.llvm.org/D112175#3482069>, @dexonsmith wrote:

> If your host toolchain has C++17, I'd be curious if the regression goes away when you switch to that mode and use `if constexpr`.
>
> I'd also be curious how this performs on your benchmark (after you fix the compiler errors / etc.):
>
>   template <class U, std::enable_if_t<U::TakesParamByValue, bool> = false>
>   static const T *reserveForParamAndGetAddressImpl(U *This, const T &Elt,
>                                                    size_t N) {
>     size_t NewSize = This->size() + N;
>     if (LLVM_LIKELY(NewSize <= This->capacity()))
>       return &Elt;
>   
>     This->grow(NewSize);
>     return &Elt;
>   }
>   template <class U, std::enable_if_t<!U::TakesParamByValue, bool> = false>
>   static const T *reserveForParamAndGetAddressImpl(U *This, const T &Elt,
>                                                    size_t N) {
>     size_t NewSize = This->size() + N;
>     if (LLVM_LIKELY(NewSize <= This->capacity()))
>       return &Elt;
>   
>     bool ReferencesStorage = false;
>     int64_t Index = -1;
>     if (!U::TakesParamByValue) {
>       if (LLVM_UNLIKELY(This->isReferenceToStorage(&Elt))) {
>         ReferencesStorage = true;
>         Index = &Elt - This->begin();
>       }
>     }
>     This->grow(NewSize);
>     return ReferencesStorage ? This->begin() + Index : &Elt;
>   }
>
> Seems like it should be mostly equivalent to your solution, but the specialization is at a lower level where it's easy to verify the logic is equivalent... and it's also still obvious how `if constexpr` would apply once we have it.

The above change does improve the results, but not quite as much as specializing at the `push_back` level.

For my toy benchmark, I'm seeing:

| **Change**                                     | **Time (ms)** | **% Reduction** |
| TOT                                            | 1190          | -               |
| specialized `reserveForParamAndGetAddressImpl` | 750           | 37.0%           |
| specialized `push_back`                        | 695           | 41.6%           |
|

I tried a couple minor variations of my benchmark and all generate similar results.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112175