[PATCH] D81515: [llvm] Release-mode ML InlineAdvisor

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/InlineAdvisor.cpp:159
+    Advisor = llvm::getReleaseModeAdvisor(M, MAM);
+#endif
     break;
----------------
davidxl wrote:
> add an assert in the #else branch?
Actually, we don't assert, rather the tryCreate caller checks the return of this function and emits an error if it didn't get an Advisor - this is the current behavior.


================
Comment at: llvm/lib/Analysis/ML/Common/MLInlineAdvisor.cpp:206
+
+  ModelRunner->set_feature(FeatureIndex::CalleeBasicBlockCount,
+                           CalleeBefore.BasicBlockCount);
----------------
davidxl wrote:
> extract the feature extraction code into a small helper?
There's a lot of little parameters to pass in that case. It'll probably be more natural when the features become more involved (multi-dimensional) to have groups of helpers like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81515