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

Tue Jul 14 14:54:03 PDT 2020

mehdi_amini added a comment.

In D82817#2151520 <https://reviews.llvm.org/D82817#2151520>, @mtrofin wrote:

> In D82817#2151481 <https://reviews.llvm.org/D82817#2151481>, @mehdi_amini wrote:
>
> > I have some concerns about TensorFlow types and datastructure being directly exposed to Analysis, and relying on macros to conditionally compile all this away.
> >  This seems to me like an indication of a missing layering, and some dependencies to cut through injection somehow.
> >
> > Can you explain what in InlineSizeEstimatorAnalysis.cpp depends on TF for example? That would be a good starting point to help me understand how to break this dependency.
> >
> > Thanks!
>
>
> In 'development' mode, InlineSizeEstimatorAnaysis uses a tensorflow model to estimate the native size of a function from some features.
>  In this upcoming patch (https://reviews.llvm.org/D83733), also in 'development' mode, we use a tensorflow model that we want to load via command line (see the patch description).
>
> Note that the TF APIs are those limited to the C API surface area - basic data types and TF_Tensor. If one builds LLVM with TF API support, and then wants to add an analysis, they may benefit from reusing this API.

I understand all this, I am still not sure we can't abstract the TF API away. I can clarify my earlier question: can you point me exactly what TF types and APIs needs to be exposed? To understand this I think that starting by `InlineSizeEstimatorAnaysis` may be a good idea, but skimming through it I only see the `FunctionFeatures` being expressed in terms of `std::vector` of various integer sizes: it isn't clear to me what from TensorFlow is used in this file right now?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82817