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.

Sign up for GitHub

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

Jump to bottom

libtensorfow-lite.a linking issue #36689

Merged
merged 2 commits into from
Feb 14, 2020
Merged

libtensorfow-lite.a linking issue #36689

merged 2 commits into from
Feb 14, 2020

Conversation

vinceab
Copy link
Contributor

@vinceab vinceab commented Feb 12, 2020

This pull request fix issues encounter while linking the generated libtensorflow-lite.a into a C/C++ application.
Multiple symbols where not defined such as:

  • type_erased.cc:(.text+0x36c): undefined reference to 'absl::Mutex::Lock()
  • rfft2d.cc:(.text+0x594): undefined reference to 'rdft2d'

absl/hash and absl/flags that where referencing absl/synchronization have been removed from the build and fft2d/fftsg2d.c has been added to the build.

This has been successfully tested against the STM32MP1 OpenSTLinux distribution.

BR
Vincent

@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Feb 12, 2020
@gbaned gbaned self-assigned this Feb 12, 2020
@gbaned gbaned added the comp:lite TF Lite related issues label Feb 12, 2020
terryheo
terryheo reviewed Feb 13, 2020
View reviewed changes
Copy link
Member

@terryheo terryheo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.
Could you also remove tensorflow/lite/tools/make/downloads/absl/absl/hash/internal/print_hash_of.cc from CORE_CC_EXCLUDE_SRCS? You can find it on master.

@vinceab
TFLite: tools: make: remove hash and flags files from the build sourc…
ccbbc58 
…es for libtensorflow-lite.a

By a file dependency game, hashtablez_sampler.cc and flags files include
the absl/synchronization/mutex.h file but all files related to absl
synchronization are not part of the build and as a result it generates a
linking issue while linking the generated libtensorflow-lite.a library into
a C/C++ application.

It is not obvious that hash and flags files are needed so simply remove them
from the list of the build sources.

Signed-off-by: Vincent ABRIOU <vincent.abriou@st.com>
@vinceab
TFLite: tools: make: add fftsg2d.c file in the build resources for li…
83d0eb1 
…btensorflow-lite.a

lite/kernels/rfft2d.cc has reference to rdft2d.
As a consequence, libtensorflow-lite.a need to include fft2d/fftsg2d.c source
in its build.

If fftsg2d.c is not part of libtensorflow-lite.a, a C/C++ application
that use the libtensorflow-lite.a static library is not able to link
with the following error:
rfft2d.cc:(.text+0x594): undefined reference to `rdft2d'

Signed-off-by: Vincent ABRIOU <vincent.abriou@st.com>
@vinceab
Copy link
Contributor Author

vinceab commented Feb 13, 2020

Patches updated.
tensorflow/lite/tools/make/downloads/absl/absl/hash/internal/print_hash_of.cc has been removed from CORE_CC_EXCLUDE_SRCS.

terryheo
terryheo approved these changes Feb 13, 2020
View reviewed changes
Copy link
Member

@terryheo terryheo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 13, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 13, 2020
tensorflow-copybara pushed a commit that referenced this pull request Feb 14, 2020
@tensorflower-gardener
Merge pull request #36689 from vinceab:master_TFLite_lib_link_issue
9977e97 
PiperOrigin-RevId: 295078069
Change-Id: Idbd7b6a45253d7e9d7ee34ab44e1d8513a982f38
@tensorflow-copybara tensorflow-copybara merged commit 83d0eb1 into tensorflow:master Feb 14, 2020
@apivovarov
Copy link
Contributor

@terryheo Can it be backported to r1.15 ?

@apivovarov apivovarov mentioned this pull request Mar 14, 2020
Merged
@terryheo
Copy link
Member

@apivovarov could you share which script is failing on r1.15?

@apivovarov
Copy link
Contributor

apivovarov commented Mar 17, 2020 •
edited
Loading

If you app uses whole libtensorflow-lite.a then the linking will fail with the following error

$ g++ -I /opt/tensorflow-1.15 \
myapp.cc \
-Wl,--whole-archive \
/opt/tensorflow-1.15/tensorflow/lite/tools/make/gen/linux_x86_64/lib/libtensorflow-lite.a \
-Wl,--no-whole-archive \
-lpthread
/opt/tensorflow-1.15/tensorflow/lite/tools/make/gen/linux_x86_64/lib/libtensorflow-lite.a(rfft2d.o): In function `tflite::ops::custom::rfft2d::Rfft2dImpl(int, int, double**, int*, double*)':
rfft2d.cc:(.text+0x730): undefined reference to `rdft2d'
collect2: error: ld returned 1 exit status

We need whole archive because app fails at runtime if we do no use the whole archive.
I guess it is nothing wrong with using all objects from libtensorflow-lite.a archive in myapp

@terryheo
Copy link
Member

confirmed. I'll work no it.

@apivovarov
Copy link
Contributor

apivovarov commented May 21, 2020 •
edited
Loading

libtensorflow-lite.a which is built from r2.1 contains .o files with main() function.

print_hash_of.o
tune_tool.o

As a result linking user's program with tflite archive results in multiple definition of 'main' (we use -Wl,--whole-archive linker option for libtensorflow-lite.a)

tensorflow/lite/tools/make/downloads/absl/absl/hash/internal/print_hash_of.cc
tensorflow/lite/experimental/ruy/tune_tool.cc

The issue was fixed in r2.2 (should it be backported to r2.1?)

fix for tune_tool.cc and print_hash_of.cc
b66f159

another fix for print_hash_of.cc
ccbbc58

@terryheo
Copy link
Member

I've created PR #39773 for r2.1 branch

@nemosupremo nemosupremo mentioned this pull request Jun 9, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terryheo terryheo terryheo approved these changes

@aselle aselle Awaiting requested review from aselle

Assignees

@gbaned gbaned

Labels
cla: yes comp:lite TF Lite related issues ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@vinceab @apivovarov @terryheo @googlebot @kokoro-team @tensorflow-copybara @gbaned