Revert "Revert "ADT: Fix reference invalidation in SmallVector...""
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Fri, 15 Jan 2021 21:53:02 +0000 (13:53 -0800)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Fri, 15 Jan 2021 22:27:48 +0000 (14:27 -0800)
commitceaf0110ff5e0c2de1f03d65d13703d34d0d5737
treea259e61a6b0031387d21b57a03664649a5ad5bf9
parentceb3cdccd0fb597659147e0f538fdee91414541e
Revert "Revert "ADT: Fix reference invalidation in SmallVector...""

This reverts commit 33be50daa9ce1074c3b423a4ab27c70c0722113a,
effectively reapplying:

260a856c2abcef49c7cb3bdcd999701db3e2af38
3043e5a5c33c4c871f4a1dfd621a8839f9a1f0b3
49142991a685bd427d7e877c29c77371dfb7634c

... with a fix to skip a call to `SmallVector::isReferenceToStorage()`
when we know the parameter had been taken by value for small, POD-like
`T`. See https://reviews.llvm.org/D93779 for the discussion on the
revert.

At a high-level, these commits fix reference invalidation in
SmallVector's push_back, append, insert (one or N), and resize
operations. For more details, please see the original commit messages.

This commit fixes a bug that crept into
`SmallVectorTemplateCommon::reserveForAndGetAddress()` during the review
process after performance analysis was done. That function is now called
`reserveForParamAndGetAddress()`, clarifying that it only works for
parameter values. It uses that knowledge to bypass
`SmallVector::isReferenceToStorage()` when `TakesParamByValue`. This is
`constexpr` and avoids adding overhead for "small enough", trivially
copyable `T`.

Performance could potentially be tuned further by increasing the
threshold for `TakesParamByValue`, which is currently defined as:
```
bool TakesParamByValue = sizeof(T) <= 2 * sizeof(void *);
```
in the POD-like version of SmallVectorTemplateBase (else, `false`).

Differential Revision: https://reviews.llvm.org/D94800
llvm/include/llvm/ADT/SmallVector.h
llvm/unittests/ADT/SmallVectorTest.cpp