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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 29 02:08:40 PDT 2022


dexonsmith added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112175



More information about the llvm-commits mailing list