[PATCH] D96854: [CodeExtractor] Enable partial aggregate arguments

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D96854#2707304 <https://reviews.llvm.org/D96854#2707304>, @ggeorgakoudis wrote:

> In D96854#2695426 <https://reviews.llvm.org/D96854#2695426>, @vsk wrote:
>
>>> for example it makes possible to extract the same region with different exclusions without creating another CodeExtractor instance,
>>
>> As extractCodeRegion mutates the original function, I assumed it was not possible to reuse a CodeExtractor instance in this way. Is there an in-tree example of CE instance reuse I can take a look at?
>
> Unfortunately, there isn't an example so far. The `CodeExtractor` instance can be re-used because the analysis of CE stays the same. The exclusion from aggregates affects only the extraction of the outlined function when calling `extractCodeRegion`.

Oh, I think I see what you meant - was it that the CE analysis cache can be re-used? The motivation there was eliminating quadratic compile-time for repeated outlining (D68616 <https://reviews.llvm.org/D68616>); with that change some cached analysis of the caller became reusable, but not the extractor instance itself.

>>> so changes in other methods (constructFunction, emitCallAndSwitchStatement) are internal and changes do not pollute this external API
>>
>> I don't quite follow. Wouldn't introducing a separate API for adding arg exclusions would also be backwards compatible?
>
> It will be backwards compatible but IMO it unnecessarily binds the CE instance to a specific way of argument creation for the outlined function.
> [snip]

I believe this would be true of any API we pick today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96854