[PATCH] D112040: [InlineAdvisor] Add fallback/format switches and negative remark processing to Replay Inliner

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/include/llvm/Analysis/InlineAdvisor.h:235
     bool tryCreate(InlineParams Params, InliningAdvisorMode Mode,
-                   StringRef ReplayFile, ReplayInlineScope ReplayScope);
+                   ReplayInlinerSettings ReplaySettings);
     InlineAdvisor *getAdvisor() const { return Advisor.get(); }
----------------
should ReplayInlinerSettings be passed as const &?


================
Comment at: llvm/include/llvm/Analysis/InlineAdvisor.h:258
 
-std::unique_ptr<InlineAdvisor> getReplayInlineAdvisor(
-    Module &M, FunctionAnalysisManager &FAM, LLVMContext &Context,
-    std::unique_ptr<InlineAdvisor> OriginalAdvisor, StringRef RemarksFile,
-    ReplayInlineScope Scope, bool EmitRemarks);
+std::unique_ptr<InlineAdvisor>
+getReplayInlineAdvisor(Module &M, FunctionAnalysisManager &FAM,
----------------
can this declaration also move to ReplayInlineAdvisor.h?


================
Comment at: llvm/include/llvm/Analysis/InlineAdvisor.h:289
 
+struct CallSiteFormat {
+  enum class Format : int {
----------------
can formatting (so callsiteformat and getCallSiteLocation) be moved to ReplayInlineAdvisor.h?

also: s/getCallSiteLocation/formatCallSiteLocation. 


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:877
 
-      auto Advice = Advisor.getAdvice(*CB, OnlyMandatory);
+      std::unique_ptr<InlineAdvice> Advice =
+          Advisor.getAdvice(*CB, OnlyMandatory);
----------------
unrelated change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112040