[PATCH] D151424: [CodeGen] Make use of MachineInstr::all_defs and all_uses. NFCI.

Mon Jun 5 00:10:35 PDT 2023

foad added a comment.

In D151424#4390193 <https://reviews.llvm.org/D151424#4390193>, @nikic wrote:

> In D151424#4390174 <https://reviews.llvm.org/D151424#4390174>, @foad wrote:
>
>>> It looks like this causes a major compile-time regression: https://llvm-compile-time-tracker.com/compare.php?from=2de54b919ba5fd9ccf37038cddfc36e97eb480af&to=5022fc2ad31b5e3211e2458347c89412b8c5ec1b&stat=instructions:u
>>
>> Could you please add https://github.com/jayfoad/llvm-project to the compile time tracker?
>
> Done!
>
>>> The implementation of these methods looks somewhat suspect: https://github.com/llvm/llvm-project/blob/7f374b6902fad9caed41284a57d573abe9ada9d1/llvm/include/llvm/CodeGen/MachineInstr.h#L732 Not sure why this is creating an `std::function`.
>>
>> That was part of my attempt at getting sane(ish) declared return types for the new functions. The first versions of this patch used `auto` and did not use `std::function`. What's so bad about `std::function` anyway?
>
> I've landed https://github.com/llvm/llvm-project/commit/1efbef4085fbe7098af4bb7013c6295ed3682cdf to remove the `std::function` use, which does seem to fully mitigate the regression. Not sure what exactly about `std::function` is so slow/unoptimizable, but it's a "well known" performance footgun (like so many things in the STL ... sigh)

Thanks and thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151424