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

Giorgis Georgakoudis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 21 22:35:55 PDT 2021


ggeorgakoudis added a comment.

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`.

>> 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. In the same sense, the `AggregateArgs` that is part of the CE constructor could be only an argument to `extractCodeRegion` since CE analysis is orthogonal to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96854



More information about the llvm-commits mailing list