[PATCH] [Inliner][NFCI] Add an InlineSite abstraction.
Philip Reames
listmail at philipreames.com
Wed Jun 24 11:51:50 PDT 2015
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/
More information about the llvm-commits
mailing list