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

[Sean Silva via Phabricator via llvm-commits]

silvas added a comment.

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

> Results of `du -k path/to/clang` followed by timing to compile llvm/lib/Target/X86/X86ISelLowering.cpp three times in a row:
>
>   116216	threshold-0/build/bin/clang-12
>          30.31 real        29.96 user         0.34 sys
>          30.10 real        29.77 user         0.31 sys
>          30.48 real        30.15 user         0.32 sys
>   116200	threshold-1-ptr/build/bin/clang-12
>          30.58 real        30.23 user         0.33 sys
>          30.17 real        29.79 user         0.37 sys
>          30.28 real        29.94 user         0.33 sys
>   116184	threshold-2-ptr/build/bin/clang-12
>          30.15 real        29.85 user         0.30 sys
>          29.98 real        29.66 user         0.31 sys
>          30.02 real        29.70 user         0.32 sys
>   116216	threshold-none/build/bin/clang-12
>          30.15 real        29.83 user         0.31 sys
>          30.16 real        29.82 user         0.34 sys
>          30.21 real        29.90 user         0.30 sys
>
> Sizes are all really close, around 116MB. Best of three runs for `user` time seems to favour `2 * sizeof(void *)` very slightly, but I'm not sure it's significant:
>
> - 29.77s: threshold of 0 (never by-value)
> - 29.79s: threshold of `sizeof(void*)`
> - 29.66s: threshold of `2 * sizeof(void*)`
> - 29.82s: no threshold (always by-value when trivially copyable)
>
> Happy to collect more data if that's useful.

Thanks! Looks in the noise. I guess inline+SROA really does clean it up very consistently :)

I don't have any objections to this patch. (It sounds like this ties into another workstream, so I'll let other folks give the final sign-off)


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

https://reviews.llvm.org/D91467