[PATCH] D86549: [MLInliner] Simplify TFUTILS_SUPPORTED_TYPES

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 11:12:24 PDT 2020


mtrofin added a comment.

This is intentional. We mainly wanted to avoid a third type alias, i.e. we have LLVM's aliases (float, doble, int32_t, etc); and tensorflow's enums (TF_FLOAT, TF_DOUBLE, TF_INT32). The uses of these 2 groups is typechecked. The one we eliminated, the string, which was only used in json, was not. Arguably, the resulting code is more robust.

In D86549#2236733 <https://reviews.llvm.org/D86549#2236733>, @ebrevdo wrote:

> This TypeSpec will no longer match specs generated by stringifying tensorflow types.  For example.
>
>   In [4]: tf.int64.name 
>   Out[4]: 'int64'
>
> meaning we will have to do the string conversion in python instead of in C++; either way we can't really get away from it.  Your choice where you want us to do it.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86549



More information about the llvm-commits mailing list