[PATCH] D28331: Improve PGO support for the new inliner

[Easwaran Raman via Phabricator via llvm-commits]

eraman marked 4 inline comments as done.
eraman added a comment.

Thanks for the comments! Will update the diff in a bit.



================
Comment at: lib/Transforms/IPO/Inliner.cpp:769-773
+  auto GetBFILambda = [&](Function &F) -> BlockFrequencyInfo & {
+    return FAM.getResult<BlockFrequencyAnalysis>(F);
+  };
 
+  function_ref<BlockFrequencyInfo &(Function &)> GetBFI(GetBFILambda);
----------------
chandlerc wrote:
> I think you can name the lambda `GetBFI` and below pass it as `{GetBFI}` if `GetBFI` doesn't work. You shouldn't need to construct a variable for the function_ref.
{GetBFI} does the trick. Thanks.


================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:2244-2246
+    // Copy original BB's block frequency to AfterCallBB
+    IFI.CallerBFI->setBlockFreq(
+        AfterCallBB, IFI.CallerBFI->getBlockFreq(OrigBB).getFrequency());
----------------
chandlerc wrote:
> The location of this makes it easy to completely miss...
> 
> I wonder, would it be better to just do this immediately when we create AfterCallBB even though we'll delete it in some cases?
I've moved it earlier as you prefer.


https://reviews.llvm.org/D28331