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