google / ml-compiler-opt

Infrastructure for Machine Learning Guided Optimization (MLGO) in LLVM.
Apache License 2.0
613 stars 92 forks source link

Bump TFLite commit #293

Closed boomanaiden154 closed 1 year ago

boomanaiden154 commented 1 year ago

This patch bumps the TFLite commit in build_tflite.sh and fixes associated fallout related to TFLite dependencies and build configurations changing.

After about 2.5 hours of debugging...

Fixes #292

boomanaiden154 commented 1 year ago

@mtrofin Maybe we want to setup CI for this somewhere? There have been multiple regressions between the TFLite version that we were at and this one related to dependencies. Catching them as they come up would probably be nice. This would probably be even more beneficial given that this TFLite build configuration doesn't seem particularly well supported. A decent amount of the patches seem to be coming from external contributors.

boomanaiden154 commented 1 year ago

Looks like this needs more work than expected. The CMake install within Tensorflow for TFLite doesn't output a find_dependency call for gemmlowp, so the CMake configuration fails. After fixing that, it seems like something either changed in how the APIs are supposed to be used or something is broke in the CMake configuration, as some header files aren't getting installed.

From the LLVM build:

/usr/bin/c++ -DCPUINFO_SUPPORTED_PLATFORM=1 -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/llvm-project/build/lib/Analysis -I/llvm-project/llvm/lib/Analysis -I/llvm-project/build/include -I/llvm-project/llvm/include -isystem /tflite/tensorflow/include -isystem /tflite/eigen/include/eigen3 -isystem /tflite/abseil-cpp/include -isystem /tflite/flatbuffers/include -isystem /tflite/gemmlowp/include/gemmlowp -isystem /tflite/ml_dtypes/src/ml_dtypes/ml_dtypes -isystem /tflite/ruy/include -isystem /tflite/cpuinfo/include -isystem /tflite/ARM_NEON_2_x86_SSE/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -DEIGEN_NEON_GEBP_NR=4 -DTFL_STATIC_LIBRARY_BUILD -std=c++17 -MD -MT lib/Analysis/CMakeFiles/LLVMAnalysis.dir/TFLiteUtils.cpp.o -MF lib/Analysis/CMakeFiles/LLVMAnalysis.dir/TFLiteUtils.cpp.o.d -o lib/Analysis/CMakeFiles/LLVMAnalysis.dir/TFLiteUtils.cpp.o -c /llvm-project/llvm/lib/Analysis/TFLiteUtils.cpp
In file included from /tflite/tensorflow/include/tensorflow/lite/core/async/async_signature_runner.h:24,
                 from /tflite/tensorflow/include/tensorflow/lite/core/interpreter.h:45,
                 from /tflite/tensorflow/include/tensorflow/lite/interpreter.h:21,
                 from /llvm-project/llvm/lib/Analysis/TFLiteUtils.cpp:25:
/tflite/tensorflow/include/tensorflow/lite/core/async/async_subgraph.h:24:10: fatal error: tensorflow/lite/core/async/interop/c/types.h: No such file or directory
   24 | #include "tensorflow/lite/core/async/interop/c/types.h"

Running

ls /tflite/tensorflow/include/tensorflow/lite/core/async/

gives

async_kernel_internal.h  async_signature_runner.h  async_subgraph.h  backend_async_kernel_interface.h  c  task_internal.h

with no interop directory, but

ls /tflite/tensorflow/src/tensorflow/tensorflow/lite/core/async/interop/c/

shows

BUILD  attribute_map.cc  attribute_map.h  attribute_map_test.cc  constants.cc  constants.h  types.cc  types.h  types_test.cc
boomanaiden154 commented 1 year ago

This should be ready once https://github.com/tensorflow/tensorflow/pull/61767 and https://github.com/tensorflow/tensorflow/pull/61768 land.

mtrofin commented 1 year ago

@mtrofin Maybe we want to setup CI for this somewhere? There have been multiple regressions between the TFLite version that we were at and this one related to dependencies. Catching them as they come up would probably be nice. This would probably be even more beneficial given that this TFLite build configuration doesn't seem particularly well supported. A decent amount of the patches seem to be coming from external contributors.

You mean a CI along the tensorflow repo. Yea, we should. I have one for the AOT scenario already using internal infra, this would be complementary. Could do it public, too, but not sure if there are triggers I could use for that.

mtrofin commented 1 year ago

reverting because https://github.com/tensorflow/tensorflow/pull/61767 didn't actually land