[PATCH] D80579: [llvm] Add function feature extraction analysis

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Analysis/ML/InlineFeaturesAnalysis.cpp:12
+  Ret.Users = ((!F.hasLocalLinkage()) ? 1 : 0) +
+              std::distance(F.user_begin(), F.user_end());
+  for (const auto &BB : F) {
----------------
Note: We could count `Uses` with `getNumUses` instead. For inlining there is probably no difference as non-callee uses cannot be inlined anyway, though I somehow feel it might be cleaner.


================
Comment at: llvm/lib/Analysis/ML/InlineFeaturesAnalysis.cpp:29
+  return Ret;
+}
----------------
This seems to be a very crude approximation. Do we have to be super fast here or would it make sense to improve on this? I mean, if I read `ConditionallyExecutedBlocks` I assume blocks not post-dominating the entry of a function, though this counts something else. Simplest example to see the difference is `if (a) s();` which would result in 3 blocks, and `ConditionallyExecutedBlocks` would say 2, though I argue it should be 1.

Either way, we should rename *and* document the Result members. `InternalCalls` seems to count calls to definitions, not to internal functions. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80579