[PATCH] D91467: ADT: Take small enough, trivially copyable T by value in SmallVector (otherwise, assert)

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D91467#2400888 <https://reviews.llvm.org/D91467#2400888>, @dexonsmith wrote:

> Note that the new assertions caught a bug in llvm/utils/TableGen/CodeGenSchedule.cpp and I found another related one by chance (not caught by these assertions though, since `emplace_back` is opaque). I'm still building, so once the tests run I assume there will be a number of other failures.

Actually, that was it. Probably helps that for small `T` the invalidation was "fixed". Maybe there would be more failures on 32-bit platforms, and/or on a bootstrap; I'll run a bootstrap before committing but don't want to spend the time if this is DOA anyway.

In D91467#2395340 <https://reviews.llvm.org/D91467#2395340>, @silvas wrote:

> Do you have any performance and binary size results?

Nope, happy to collect something though. What do you think we need to see?

Note that right now the "fix invalidation" and "assert if invalid" are tied together in the same patch. Of course they're separable, but I'd be tempted to keep them together if there's a path to landing both. But if the optimization is DOA, I imagine we'd still want the assertion (and fixes for any extra assertion failures).


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

https://reviews.llvm.org/D91467