[all-commits] [llvm/llvm-project] ceaf01: Revert "Revert "ADT: Fix reference invalidation in...

Duncan P. N. Exon Smith via All-commits all-commits at lists.llvm.org
Fri Jan 15 14:30:26 PST 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ceaf0110ff5e0c2de1f03d65d13703d34d0d5737
      https://github.com/llvm/llvm-project/commit/ceaf0110ff5e0c2de1f03d65d13703d34d0d5737
  Author: Duncan P. N. Exon Smith <dexonsmith at apple.com>
  Date:   2021-01-15 (Fri, 15 Jan 2021)

  Changed paths:
    M llvm/include/llvm/ADT/SmallVector.h
    M llvm/unittests/ADT/SmallVectorTest.cpp

  Log Message:
  -----------
  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




More information about the All-commits mailing list