[PATCH] D104251: Remove ML inlining model artifacts.

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/cmake/modules/TensorFlowCompile.cmake:1
 # Ensure the ${model} is available at ${final_path}.
 #
----------------
This file shouldn't have anything inlining specific.


================
Comment at: llvm/cmake/modules/TensorFlowCompile.cmake:3
 #
-function(tfgetmodel model final_path)
+function(get_absolute_path model final_path)
   if (IS_ABSOLUTE ${model})
----------------
can you give it a more specific name, to avoid potential name collision - e.g. get_tf_model_abs_path or smth like that


================
Comment at: llvm/cmake/modules/TensorFlowCompile.cmake:12
 
+# Generate a saved inlining model.
+function(generate_inlining_model python model generate_inlining_model_py)
----------------
Add in the comment ".. for test"


================
Comment at: llvm/cmake/modules/TensorFlowCompile.cmake:13
+# Generate a saved inlining model.
+function(generate_inlining_model python model generate_inlining_model_py)
+  get_absolute_path(${model} LLVM_ML_MODELS_ABSOLUTE)
----------------
the name could suggest it's a mock model. 


================
Comment at: llvm/lib/Analysis/models/generate_inlining_model.py:1
+"""Generate a mock model for testing the ML Inline Advisor.
+
----------------
make this file inlining-independent, and the interface something that can be provided via configuration, so we can eventually factor out that config somehow in a language-independent way, and share it between c++ and python.

It's fine if the configuration is just another python that defines the features and their tensor shape, no need for a configuration language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104251