[PATCH] D109448: [IRSim][IROutliner] Treating Called Functions as Values and Outlining Indirect Calls

[Jessica Paquette via Phabricator via llvm-commits]

paquette accepted this revision.
paquette added a comment.
This revision is now accepted and ready to land.

I mostly have nits at this point, so I think this is more or less good to go.



================
Comment at: llvm/include/llvm/Analysis/IRSimilarityIdentifier.h:516
       Function *F = CI.getCalledFunction();
-      if (!F || CI.isIndirectCall() || !F->hasName())
+      if (CI.isIndirectCall() && !EnableIndirectCalls)
+        return Illegal;
----------------
Pull `CI.isIndirectCall()` into a variable?


================
Comment at: llvm/include/llvm/Transforms/IPO/IROutliner.h:370
+        return false;
+      if (!F && !CI.isIndirectCall())
         return false;
----------------
Pull `CI.isIndirectCall()` into a variable?


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:115
 
-static StringRef getCalledFunctionName(CallInst &CI) {
-  assert(CI.getCalledFunction() != nullptr && "Called Function is nullptr?");
----------------
Why not replace this with a `getCalleeName` function? That could also assert that you actually did set the callee name as well.


================
Comment at: llvm/lib/Analysis/IRSimilarityIdentifier.cpp:186
 
   // If the instructions are functions, we make sure that the function name is
   // the same.  We already know that the types are since is isSameOperationAs is
----------------
Shouldn't this comment say "if the instructions are function calls"?


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

https://reviews.llvm.org/D109448