[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#1305662, @jdoerfert wrote:

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


Right that makes sense, thanks! I do not have any strong opinions about name or place either. It would be great to get a few more people's thoughts on this.

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

Sounds good.


Repository:
  rL LLVM

https://reviews.llvm.org/D54498