[PATCH] D85674: [MLInliner] In development mode, obtain the output specs from a file

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked 4 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp:656-669
+  ModelUnderTrainingRunner *MUTRPtr = nullptr;
   bool IsDoingInference = false;
   if (TFModelUnderTrainingPath.empty())
     Runner.reset(new NoInferenceModelRunner(Ctx));
   else {
-    Runner = std::make_unique<ModelUnderTrainingRunner>(
+    auto MUTR = std::make_unique<ModelUnderTrainingRunner>(
         Ctx, TFModelUnderTrainingPath);
----------------
yundiqian wrote:
> mtrofin wrote:
> > yundiqian wrote:
> > > I'm a bit confused here, why we need both MUTR and Runner in the TrainingLogger? Are they just the same? (why not using ModelRunner instead of MUTR in all TrainingLogger functions?)
> > Runner is generically typed as MLModelRunner, which could be either a ModelUnderTrainingRunner or the NoInferenceModelRunner, see lines 659 and 661.
> ha, I see, but the APIs here looks quite confusing to me...Would it be better to add some APIs in NoInferenceModelRunner such that we don't need two different ModelRunner here?
> 
> 
you mean no-op apis to NoInferenceModelRunner? we can look into it after this CL, so we make sure the whole functionality would work in that case (and see what the usage would look like, wouldnit actually be simpler, etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85674