[PATCH] [Inliner][NFCI] Add an InlineSite abstraction.

[Philip Reames]

Comments inline.


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:66
@@ -65,1 +65,3 @@
 namespace {
+class InlineSite {
+  Function *CalledFunction;
----------------
I'm not convinced of the need for a new abstraction here.  What does this really add over having a function which takes a call site and extracts the underlying callee from a statepoint and the surface callee from a normal call?  An getUnwrappedCallee(CallSite CS) function seems to serve the same purpose with much less complexity.

Even if I accept the need for an extra abstraction, I *strongly* object to the duplication of the underlying fields.  Add proxying access if needed, but please do not duplicate each field from the underlying call site.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:119
@@ +118,3 @@
+
+static bool InlineFunctionImpl(InlineSite IS, InlineFunctionInfo &IFI,
+                               bool InsertLifetime, Value *&ReturnVal,
----------------
Placement wise, this should be below the existing InlineFunction code.

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:131
@@ +130,3 @@
+
+  Instruction *TheCall = CS.getInstruction();
+  if (!TheCall->use_empty()) {
----------------
I'm confused by this bit.  It seems like possibly an unrelated refactoring?  Or is this the statepoint related bits for gc.result?

================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1367
@@ -1264,13 +1366,3 @@
 
-    // If the return instruction returned a value, replace uses of the call with
-    // uses of the returned value.
-    if (!TheCall->use_empty()) {
-      ReturnInst *R = Returns[0];
-      if (TheCall == R->getReturnValue())
-        TheCall->replaceAllUsesWith(UndefValue::get(TheCall->getType()));
-      else
-        TheCall->replaceAllUsesWith(R->getReturnValue());
-    }
-    // Since we are now done with the Call/Invoke, we can delete it.
-    TheCall->eraseFromParent();
+    ReturnVal = Returns[0]->getReturnValue();
 
----------------
Unintentional diff?

http://reviews.llvm.org/D10631

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/