[PATCH] D119507: [MLGO] Use TFLite in 'development mode'

[Eugene via Phabricator via llvm-commits]

ebrevdo requested changes to this revision.
ebrevdo added a comment.
This revision now requires changes to proceed.

Some small items.



================
Comment at: llvm/CMakeLists.txt:897
 set(TENSORFLOW_C_LIB_PATH "" CACHE PATH "Path to TensorFlow C library install")
-if (TENSORFLOW_C_LIB_PATH)
+set(LLVM_HAVE_TFLITE "" CACHE BOOL "Use tflite")
+if (LLVM_HAVE_TFLITE)
----------------
Should this be LLVM_USE_TFLITE?


================
Comment at: llvm/CMakeLists.txt:904
+  include_directories(${LLVM_PROTOBUF_OUT_DIR})
+elseif (TENSORFLOW_C_LIB_PATH)
   find_library(tensorflow_c_api tensorflow PATHS ${TENSORFLOW_C_LIB_PATH}/lib NO_DEFAULT_PATH REQUIRED)
----------------
do you plan to remove this in future?


================
Comment at: llvm/include/llvm/Config/llvm-config.h.cmake:102
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
----------------
plan to remove this in future?


================
Comment at: llvm/lib/Analysis/TFLiteUtils.cpp:103
+    : Input(InputSpecs.size()), Output(OutputSpecsSize) {
+  tflite::StderrReporter ErrorReporter;
+  SmallVector<char, 128> TFLitePathBuff;
----------------
Suggest subclassing tflite::StatefulErrorReporter like [here](https://github.com/ochi96/mediapipe_simplified/blob/2b6070b69d907298bf3c87f486c74689765bc9a3/mediapipe/util/tflite/error_reporter.cc) so that you can check error status by looking at reporter.message() after the initialization and after running inference.  you can even add a bool like has_error which will be cheaper (no copy).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119507