Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adjust abseil version to be compatible between cmake tensorflowlite and bazel #50597

Merged

Conversation

kruglov-dmitry
Copy link
Contributor

At the moment version of libabseil library defined in bazel and cmake scripts are different:

Bazel will add those namespace prefix lts_2020_09_23 and cmake this one lts_2020_02_25.

In case you compile tensorflowlite library via cmake and build remaining libraries for tensorflow lite using bazel (for example libmetal-delegate) you can't use link them later as you will get unresolved symbols:

Unresolved symbol:

absl::lts_2020_09_23::UnknownError(absl::lts_2020_09_23::string_view), 
referenced from: tflite::gpu::metal::(anonymous namespace)::AllocateTensorMemory(
id<MTLDevice>,tflite::gpu::StrongShape<(tflite::gpu::Layout)12> const&, tflite::gpu::TensorDescriptor const&, void const*, id<MTLBuffer> __autoreleasing*, id<MTLTexture> __autoreleasing*) 
in libmetal_spatial_tensor.a(metal_spatial_tensor.o)

But, for libraries compiled by cmake (and used in tf lite):

nm ./tensorflow/lite/tf_build/_deps/abseil-cpp-build/absl/status/libabsl_status.a | grep UnknownError
0000000000003110 T __ZN4absl14lts_2020_02_2512UnknownErrorENS0_11string_viewE

absl::lts_2020_09_23::UnknownError(absl::lts_2020_09_23::string_view
vs
absl::lts_2020_02_25::UnknownError string_view

@google-cla google-cla bot added the cla: yes label Jul 4, 2021
@thaink thaink requested a review from terryheo July 5, 2021 03:32
@terryheo
Copy link
Member

terryheo commented Jul 5, 2021

We don't accept new change for 2.5 branch except cherry-picks. Could you check if the cherry-pick of 8348b34 resolves the issue?

@gbaned gbaned self-assigned this Jul 5, 2021
@gbaned gbaned added comp:lite TF Lite related issues size:XS CL Change Size: Extra Small labels Jul 5, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jul 5, 2021
@gbaned gbaned assigned mihaimaruseac and unassigned gbaned Jul 5, 2021
@kruglov-dmitry
Copy link
Contributor Author

I've cloned TF from scratch, switch on v2.5.0 and run

git cherry-pick 8348b348ac509c70a8730416be773781ddd6e2ed

After that file

  • tensorflow/lite/tools/cmake/modules/abseil-cpp.cmake
    contains this hash:
    GIT_TAG 997aaf3a28308eba1b9156aa35ab7bca9688e9f6
    which resolved to
git describe --tags
20210324.0
  • third_party/absl/workspace.bzl
    contains this hash:
    ABSL_COMMIT = "6f9d96a1f41439ac172ee2ef7ccd8edf0e5d068c"
    which is resolved to
git describe --tags
20200923.3

i.e. unfortunately it will still lead to mismatched symbols.

Just in case I've compiled libs for usage of metal delegate bia bazel and build tensorflowlite itself via cmake.

During compilation I've got unresolved symbols:

Undefined symbols for architecture arm64:
  "absl::lts_2020_09_23::UnknownError(absl::lts_2020_09_23::string_view)", referenced from:
      tflite::gpu::metal::(anonymous namespace)::AllocateTensorMemory(id<MTLDevice>, tflite::gpu::StrongShape<(tflite::gpu::Layout)12> const&, tflite::gpu::TensorDescriptor const&, void const*, id<MTLBuffer> __autoreleasing*, id<MTLTexture> __autoreleasing*) in libmetal_spatial_tensor.a(metal_spatial_tensor.o)
find tf_cherry_pick/_deps/ -iname '*.a' -exec nm -A {} \; | grep UnknownError
tf_cherry_pick/_deps//libabsl_status.a:status.cc.o: 0000000000002dc4 T __ZN4absl12lts_2021032412UnknownErrorENS0_11string_viewE

and tensorflowlite itself contains symbols from abseil 20210324 namespace

nm tf_cherry_pick/libtensorflow-lite.a | grep 20210324 | grep UnknownError
                 U __ZN4absl12lts_2021032412UnknownErrorENS0_11string_viewE

@mihaimaruseac
Copy link
Collaborator

We can merge this but only when we do a patch release on TF 2.5

@mihaimaruseac mihaimaruseac added the waiting for patch release PR will be reviewed and merged only if we do a patch release since PR is not on master branch label Jul 5, 2021
@terryheo
Copy link
Member

terryheo commented Jul 8, 2021

Linking Bazel generated binary with CMake looks strange a use-case.
If you can use the Bazel, why do you need to use CMake?

@kruglov-dmitry
Copy link
Contributor Author

Thats a good question!

Bazel was a new beast for our team so we have started with old(?) good(?) cmake.

Along the way, specifically when we try to adopt delegates, we realise that not all artefacts are buildable by it,

  • so I've to build missing dependencies using bazel and tried to figure out why they not match each other.

ps Thats probably offtopic here, but I would be more then happy to see how to
properly generate coreml related dependencies, as I've end up by building coremltool libs with specific version of protobuf
and patch few things to include missing symbols into libraries in order to get it working on device.

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Aug 6, 2021
@mihaimaruseac mihaimaruseac merged commit 47beb4c into tensorflow:r2.5 Aug 6, 2021
PR Queue automation moved this from Approved by Reviewer to Merged Aug 6, 2021
@kruglov-dmitry kruglov-dmitry deleted the v2.5.0-sync-abseil-cmake-bazel branch August 6, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:lite TF Lite related issues size:XS CL Change Size: Extra Small waiting for patch release PR will be reviewed and merged only if we do a patch release since PR is not on master branch
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants