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

[Yundi Qian via Phabricator via llvm-commits]

yundiqian added inline comments.


================
Comment at: llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp:63-67
+static cl::opt<std::string> TFOutputSpecOverride(
+    "ml-inliner-output-spec-override", cl::Hidden,
+    cl::desc("Override the path to the output spec json file. See "
+             "-ml-inliner-model-under-training documentation for the "
+             "specification of that file."));
----------------
what's the motivation of providing this override alternative?


================
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);
----------------
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?)


================
Comment at: llvm/lib/Analysis/models/inliner/output_spec.json:1-13
+[
+    {
+        "logging_name": "inlining_decision",
+        "tensor_spec": {
+            "name": "StatefulPartitionedCall",
+            "port": 0,
+            "type": "int64",
----------------
where is this file being used? Is it used when running default policy?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85674