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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

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.

Constant propagation is an obvious use case for this. What other transformations do you envision using this?

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



================
Comment at: lib/IR/AbstractCallSite.cpp:80
+  // A collection of known broker functions.
+  enum KnownBrokerFunctions {
+    KBF_UNKNOWN,
----------------
Could this be an enum class? I think you could drop the default case in the switch later on, given you handle all cases. The compiler will warn/error on not handled cases.


================
Comment at: lib/IR/AbstractCallSite.cpp:95
+  // First check if we did indeed find a known broker function or not.
+  if (BrokerFn == KBF_UNKNOWN) {
+    // If we cannot create a call back for this use we invalidate the abstract
----------------
Is there a reason this cannot be part of the switch below?


Repository:
  rL LLVM

https://reviews.llvm.org/D54498