[PATCH] D54498: AbstractCallSite -- A unified interface for (in)direct and callback calls

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D54498#1298538, @jdoerfert wrote:

> Thanks for the comments!
>
> In https://reviews.llvm.org/D54498#1298137, @fhahn wrote:
>
> > I think using this in IPSCCP would be a great showcase too, given that it is more powerful (and enabled in the default pipeline). `lib/Transforms/Scalar/SCCP.cpp` contains the solver that is used for both per function SCCP and IPSCCP.
>
>
> SCCP is certainly on my list, but one of the IPOs I haven't looked into yet.
>
> > Constant propagation is an obvious use case for this. What other transformations do you envision using this?
>
> I have however a version of argument promotion (which turns out to be **really** important wrt parallel codes).
>  I also have extended the attribute deduction pass in various ways with an older version of the abstract/transitive/callback call sites, see here <https://reviews.llvm.org/D50146> and here <https://reviews.llvm.org/D50125>.
>  Initial results from the prototype implementations can be found in the talk and the paper (see commit message). Basically, they are as good as the dedicated transformations we implemented and evaluated here <http://compilers.cs.uni-saarland.de/people/doerfert/par_opt18.pdf>.


FWIW TransitiveCallSite seems like a slightly better name to me than AbstractCallSite, as the CallSite class is already an abstraction over different kinds of calls. As for the location of the class, if it is tied to an analysis that gathers the transitive call sites, having it in lib/Analysis/ might make sense initially as well.

As for testing/benchmarking this, is there an easy way to test this/gather statistics on real benchmarks? (e.g. via the test-suite)

>>> With this commit two known broker functions that invoke a callback are
>>>  identified by their name. While this is not the perfect way to handle
>>>  known broker functions, it should suffice for now.
>> 
>> I do not have any other concrete suggestions right now, but tying code in lib/IR to string names of library calls does not seem ideal. I guess one alternative would be to have some sort of !broker attribute/metadata and then either have the frontend generate it and/or a pass to infer it. Although that might be over the top.
> 
> I don't think that is over the top. I have such metadata already. While I annotate it automatically based on the name (in the middle end for now), I want the front end to do it. We have some name based resolution already and we could hide it behind a flag (the current patch is not executed by default!). I also want to support user-provided callbacks either through annotations or identified by an analysis.
> 
>  ----
> 
> Overall, what do you think about think about this?

I like the overall approach and think anything that extends the scope of IPOs is great, as it increases the incentive to work on them.


Repository:
  rL LLVM

https://reviews.llvm.org/D54498