[PATCH] D82817: [llvm] Native size estimator for training -Oz inliner

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp:217
+
+void IRToNativeSizeLearning::FunctionFeatures::fillTensor(int32_t *Ptr) const {
+  int Pos = 0;
----------------
To avoid this translation, the Features can be a wrapper of the tensor array -- with each field accessed with a symbolic index.


================
Comment at: llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp:263
+    return None;
+  auto Features = IRToNativeSizeLearning::getFunctionFeatures(
+      const_cast<Function &>(F), FAM);
----------------
Can we make getFunctionFeatures directly return the filled tensor -- or at least provide a wrapper? There is no need to expose the TF details with the inline sequence here.


================
Comment at: llvm/lib/Analysis/InlineSizeEstimatorAnalysis.cpp:268
+  std::vector<TF_Tensor *> Output{nullptr};
+  if (!Evaluator->evaluate(Output))
+    return None;
----------------
Code from line 268 to 271 can probably be wrapped in a single wrapper function to hide TF details including Tensor delete


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82817