[PATCH] D69736: Keep import function list for inlinee profile update

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Nice catch on the dropped import lists!



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:876
   }
-  InlineFunctionInfo IFI(nullptr, &GetAC);
+  InlineFunctionInfo IFI(nullptr, &GetAC, nullptr, nullptr, nullptr, false);
   if (InlineFunction(CS, IFI)) {
----------------
Document constant parameters.


================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1723
+    if (IFI.CallerBFI != nullptr && IFI.CalleeBFI != nullptr &&
+        IFI.UpdateProfile)
       // Update the BFI of blocks cloned into the caller.
----------------
Can you split this into a separate patch (with a test) since it isn't related to the headline of this patch?  Are we currently getting incorrect profiles without this fix?

Suggest pulling the if (IFI.UpdateProfile) checks out into a single if for clarity.


================
Comment at: llvm/test/Transforms/SampleProfile/inline-callee-update.ll:1
+; facebook T56498894: Make sure Import GUID list for ThinLTO properly maintained while update function's entry count for inlining
+
----------------
Is this intended to be here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69736