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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In https://reviews.llvm.org/D54498#1305504, @fhahn wrote:

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


There are a few things to consider but at the end I will not fight anybody for a certain name or source location. Things that led me to
do this design included:

- Abstract call sites, as implemented here, are not *only* transitive call sites but can be transitive **or** direct call sites.
- The call site abstraction is going away soon [0]. Afterwards, the only abstraction will be this one on top of the new `CallBase` class.
- The analysis to identify callback functions doesn't exist yet. When it does, I would probably only annotate the IR and use these annotations in the constructor of AbstractCallSite. This way we can easily pass uses to the constructor and do not need to a dedicated pass that provides both direct and transitive calls (separately or unified through the AbstractCallSite abstraction).



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

Yes and no. The test suite doesn't contain/exhibit OpenMP/Pthread parallelism (yet). Thus, we cannot create abstract call sites that way.
If we would add the names of sequential callback functions we could test those. We are also looking into options to put OpenMP into the
test suite. Finally, I did test/benchmark a very similar implementation (less clean) on various OpenMP programs for the two papers and the
LLVM-Dev Talk [1]

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

Good to hear. I totally agree and I'm currently working on a proper AttributeDeduction (see [2]) for which I hope to push the first patch up for review soon.

[0] RFC: Removing TerminatorInst, simplifying calls <https://lists.llvm.org/pipermail/llvm-dev/2018-May/123407.html>
[1] LLVM-Dev Talk 2018: Slides <https://llvm.org/devmtg/2018-10/slides/Doerfert-Johannes-Optimizing-Indirections-Slides-LLVM-2018.pdf> Video <https://youtu.be/zfiHaPaoQPc>
[2] RFC: "Properly" Derive Function/Argument/Parameter Attributes <https://lists.llvm.org/pipermail/llvm-dev/2018-September/125824.html>


Repository:
  rL LLVM

https://reviews.llvm.org/D54498