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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/ML/InlineFeaturesAnalysis.cpp:29
+  return Ret;
+}
----------------
jdoerfert wrote:
> 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. 
Agreed it's crude. We'd want to improve the feature set subsequently in a number of ways, I left a FIXME here with your example, and also renamed / documented members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80579