[PATCH] D89587: [CGSCC] Detect devirtualization in more cases

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

In D89587#2400646 <https://reviews.llvm.org/D89587#2400646>, @aeubanks wrote:

> In D89587#2398531 <https://reviews.llvm.org/D89587#2398531>, @asbirlea wrote:
>
>> Could you clarify why a map and not a SmallVector?
>>
>> I was expecting the indirect calls to be added by all CGSCC passes, e.g. `lib/Transforms/IPO/Inliner.cpp:750`. Adding the calls in `updateCG...` instead, introduces a single point of failure so less chances of having passes forget to add them; the one thing I'm not clear is if the `updateCG...` call may be added too late in a CGSCC pass; for the inliner, it seems fine (thank you for adding the tests) but I'm not clear if this will hold for other passes. I don't have a use case now, just making a note to verify this for coroutines.
>> The check that all accesses are still indirect on the return looks good.
>
> It is definitely possible to have a call to `updateCG...` miss a devirtualization in a CGSCC pass if an indirect call is introduced and then promoted before the call to `updateCG...`, but the inliner looks ok. I think other CGSCC passes don't really devirtualize much. As long as we call `updateCG...` after passes that can devirtualize (inliner, function passes like instcombine, etc) we should be good. And we can always revisit if we find missed cases.

SG. Can you make a note/comment on this in the update function where adding IndirectVH?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89587