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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked 2 inline comments as done.
mtrofin added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ML/InlineFeaturesAnalysis.h:32
+    int64_t DirectCallsToDefinedFunctions = 0;
+  };
+  Result run(const Function &F, FunctionAnalysisManager &FAM);
----------------
jdoerfert wrote:
> mtrofin wrote:
> > jdoerfert wrote:
> > > Nit: We now count `Uses`, not `Users`.
> > > Nit: `uint64_t` as all are counts.
> > Done the rename part. 
> > 
> > Re. signedness / unsignedness: initially, I had them as size_t, but:
> > - tensorflow works with signed values, and it is the main consumer of this data
> > - one other use in the (next CL) MLInlineAdvisor is to delta-update some module-wide values (like total nr of edges in a module). With uint64_t, we need to be very careful with how that update happens (subtractions can silently overflow).
> > 
> > With signed int, that's not a problem.
> Fair enough. 
Is the patch good to go?

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80579