[PATCH] D17864: [PGO] Promote indirect calls to conditional direct calls with value-profile

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: include/llvm/ProfileData/InstrProf.h:160
@@ +159,3 @@
+/// Return the modified name for function \c F suitable to be
+/// used the key for profile lookup in LTO compilation.
+std::string getPGOFuncNameLTO(const Function &F);
----------------
I expect lengthy description of the naming, why the difference between LTO and non-LTO. If it described somewhere else, please insert a reference to it.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:126
@@ -106,2 +125,3 @@
 namespace {
+
 class PGOInstrumentationGen : public ModulePass {
----------------
Remove

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:1140
@@ +1139,3 @@
+      BasicBlock::iterator It(NewInst);
+      Instruction *NextInst = &*(++It);
+      NewInst = new BitCastInst(NewInst, CallRetType, "", NextInst);
----------------
xur wrote:
> silvas wrote:
> > std::next (or llvm::next, if we can't use std::next yet).
> > 
> > I.e.
> > 
> > ```
> > if (FuncRetType != CallRetType)
> >   NewInst = new BitCastInst(NewInst, CallRetType, "", std::next(NewInst));
> > ```
> > 
> > Also, I see you are using IRBuilder above. Any reason why IRBuilder is not preferred for the IR building later in the function?
> No particular reason that I don't use IRbuilder. The inserted instructions are not in different BBs (not with the original call inst). I just thought it's convenient to change the code stream directly.
The IRBuilder has the nice property that you can supply a name for the instruction and it'll show up only in debug build.


http://reviews.llvm.org/D17864