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
Fix Windows shared library build for CMake #47621
Fix Windows shared library build for CMake #47621
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
tensorflow/lite/CMakeLists.txt
Outdated
@@ -415,6 +415,11 @@ target_link_libraries(tensorflow-lite | |||
ruy | |||
${TFLITE_TARGET_DEPENDENCIES} | |||
) | |||
|
|||
if (BUILD_SHARED_LIBS) | |||
list(APPEND TFLITE_TARGET_PUBLIC_OPTIONS "TFL_SHARED_LIBRARY_BUILD") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already use TFL_COMPILE_LIBRARY to build windows dll.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFL_COMPILE_LIBRARY switch between dllexport and dllimport. Neither could be applied if static library build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when BUILD_SHARED_LIBS is used, what's the expected definition of TFL_CAPI_EXPORT?
Do we still need to check TFL_COMPILE_LIBRARY?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. TFL_CAPI_EXPORT resolves to dllexport if TFL_COMPILE_LIBRARY otherwise to dllimport.
Could you also provide cmake commands you used? Is there anything special? |
I use lite through add_subdirectory. There are not something special for cmake. |
tensorflow/lite/c/c_api_types.h
Outdated
@@ -30,6 +30,9 @@ extern "C" { | |||
#ifdef SWIG | |||
#define TFL_CAPI_EXPORT | |||
#else | |||
#ifdef TFL_STATIC_LIBRARY_BUILD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to use #elif defined()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
MSVC build support with CMake