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

Remove duplicated linking of libraries. #4890

Closed
wants to merge 7 commits into from
Closed

Remove duplicated linking of libraries. #4890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
31c53f9
Remove unused cmake files and statements.
Xreki Oct 12, 2017
ea36196
Merge branch 'develop' into fix_ios_linking
Xreki Oct 16, 2017
a7cc73a
Remove duplicate DEPS.
Xreki Oct 16, 2017
46e79bc
Remove duplicated links on Mac and iOS.
Xreki Oct 17, 2017
9936847
Merge branch 'develop' into fix_ios_linking
Xreki Oct 27, 2017
5a8a81c
Add the global link of gflags and glog back, because so many modules …
Xreki Oct 27, 2017
14059a6
Refine the ambiguity comment in ios.cmake.
Xreki Oct 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
582 changes: 0 additions & 582 deletions cmake/FindGflags.cmake
View file
Open in desktop

This file was deleted.

24 changes: 0 additions & 24 deletions cmake/FindGlog.cmake
View file
Open in desktop

This file was deleted.

8 changes: 5 additions & 3 deletions cmake/cross_compiling/ios.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ endif()

set(IOS_COMPILER_FLAGS "${XCODE_IOS_PLATFORM_VERSION_FLAGS} ${XCODE_IOS_BITCODE_FLAGS}")

# Hidden visibilty is required for cxx on iOS
# FIXME(liuyiqun01): Hidden visibilty is required for c++ on iOS,
# but with -fvisibility=hidden external library warp-ctc fails, so remove it temporarily.
set(CMAKE_C_FLAGS "${IOS_COMPILER_FLAGS} ${CMAKE_C_FLAGS}" CACHE STRING "C flags")
set(CMAKE_CXX_FLAGS "${IOS_COMPILER_FLAGS} -fvisibility-inlines-hidden ${CMAKE_CXX_FLAGS}" CACHE STRING "CXX flags")

Expand All @@ -266,10 +267,10 @@ if(IOS_USE_VECLIB_FOR_BLAS)

if(VECLIB_FOUND)
if(VECLIB_INC_DIR MATCHES "^/System/Library/Frameworks/vecLib.framework.*")
set(IOS_LINK_FLAGS ${IOS_LINK_FLAGS} -lcblas "-framework vecLib")
set(IOS_LINK_FLAGS "${IOS_LINK_FLAGS} -framework vecLib")
message(STATUS "Found standalone vecLib.framework")
else()
set(IOS_LINK_FLAGS ${IOS_LINK_FLAGS} -lcblas "-framework Accelerate")
set(IOS_LINK_FLAGS "${IOS_LINK_FLAGS} -framework Accelerate")
message(STATUS "Found vecLib as part of Accelerate.framework")
endif()

Expand All @@ -278,6 +279,7 @@ endif()

set(CMAKE_C_LINK_FLAGS "${IOS_LINK_FLAGS} ${CMAKE_C_LINK_FLAGS}")
set(CMAKE_CXX_LINK_FLAGS "${IOS_LINK_FLAGS} ${CMAKE_CXX_LINK_FLAGS}")
set(CMAKE_EXE_LINKER_FLAGS "${IOS_LINK_FLAGS} ${CMAKE_EXE_LINKER_FLAGS}")

set(CMAKE_PLATFORM_HAS_INSTALLNAME 1)
if(NOT IOS_ENABLE_BITCODE)
Expand Down
43 changes: 25 additions & 18 deletions cmake/generic.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@
include_directories(${CMAKE_CURRENT_BINARY_DIR})

if(NOT APPLE AND NOT ANDROID)
find_package(Threads REQUIRED)
link_libraries(${CMAKE_THREAD_LIBS_INIT})
set(CMAKE_CXX_LINK_EXECUTABLE "${CMAKE_CXX_LINK_EXECUTABLE} -ldl -lrt")
find_package(Threads REQUIRED)
link_libraries(${CMAKE_THREAD_LIBS_INIT})
set(CMAKE_CXX_LINK_EXECUTABLE "${CMAKE_CXX_LINK_EXECUTABLE} -ldl -lrt")
endif(NOT APPLE AND NOT ANDROID)

function(merge_static_libs TARGET_NAME)
Expand Down Expand Up @@ -180,12 +180,13 @@ function(cc_library TARGET_NAME)
set(multiValueArgs SRCS DEPS)
cmake_parse_arguments(cc_library "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
if (cc_library_SRCS)
if (cc_library_SHARED OR cc_library_shared) # build *.so
if(cc_library_SHARED OR cc_library_shared) # build *.so
add_library(${TARGET_NAME} SHARED ${cc_library_SRCS})
else()
add_library(${TARGET_NAME} STATIC ${cc_library_SRCS})
endif()
if (cc_library_DEPS)
if(cc_library_DEPS)
list(REMOVE_DUPLICATES cc_library_DEPS)
add_dependencies(${TARGET_NAME} ${cc_library_DEPS})
target_link_libraries(${TARGET_NAME} ${cc_library_DEPS})
endif()
Expand Down Expand Up @@ -226,6 +227,9 @@ function(cc_test TARGET_NAME)
set(oneValueArgs "")
set(multiValueArgs SRCS DEPS)
cmake_parse_arguments(cc_test "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
if(cc_test_DEPS)
list(REMOVE_DUPLICATES cc_test_DEPS)
endif()
add_executable(${TARGET_NAME} ${cc_test_SRCS})
target_link_libraries(${TARGET_NAME} ${cc_test_DEPS} gtest gtest_main)
add_dependencies(${TARGET_NAME} ${cc_test_DEPS} gtest gtest_main)
Expand All @@ -240,12 +244,13 @@ function(nv_library TARGET_NAME)
set(multiValueArgs SRCS DEPS)
cmake_parse_arguments(nv_library "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
if(nv_library_SRCS)
if (nv_library_SHARED OR nv_library_shared) # build *.so
if(nv_library_SHARED OR nv_library_shared) # build *.so
cuda_add_library(${TARGET_NAME} SHARED ${nv_library_SRCS})
else()
cuda_add_library(${TARGET_NAME} STATIC ${nv_library_SRCS})
cuda_add_library(${TARGET_NAME} STATIC ${nv_library_SRCS})
endif()
if (nv_library_DEPS)
if(nv_library_DEPS)
list(REMOVE_DUPLICATES nv_library_DEPS)
add_dependencies(${TARGET_NAME} ${nv_library_DEPS})
target_link_libraries(${TARGET_NAME} ${nv_library_DEPS})
endif()
Expand All @@ -268,7 +273,7 @@ function(nv_library TARGET_NAME)
endfunction(nv_library)

function(nv_binary TARGET_NAME)
if (WITH_GPU)
if(WITH_GPU)
set(options "")
set(oneValueArgs "")
set(multiValueArgs SRCS DEPS)
Expand All @@ -282,11 +287,14 @@ function(nv_binary TARGET_NAME)
endfunction(nv_binary)

function(nv_test TARGET_NAME)
if (WITH_GPU AND WITH_TESTING)
if(WITH_GPU AND WITH_TESTING)
set(options "")
set(oneValueArgs "")
set(multiValueArgs SRCS DEPS)
cmake_parse_arguments(nv_test "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
if(nv_test_DEPS)
list(REMOVE_DUPLICATES nv_test_DEPS)
endif()
cuda_add_executable(${TARGET_NAME} ${nv_test_SRCS})
target_link_libraries(${TARGET_NAME} ${nv_test_DEPS} gtest gtest_main)
add_dependencies(${TARGET_NAME} ${nv_test_DEPS} gtest gtest_main)
Expand Down Expand Up @@ -402,10 +410,10 @@ function(paddle_protobuf_generate_cpp SRCS HDRS)
set(${SRCS})
set(${HDRS})

if (MOBILE_INFERENCE)
set(EXTRA_FLAG "lite:")
if(MOBILE_INFERENCE)
set(EXTRA_FLAG "lite:")
else()
set(EXTRA_FLAG "")
set(EXTRA_FLAG "")
endif()

foreach(FIL ${ARGN})
Expand All @@ -420,14 +428,13 @@ function(paddle_protobuf_generate_cpp SRCS HDRS)
add_custom_command(
OUTPUT "${_protobuf_protoc_src}"
"${_protobuf_protoc_hdr}"

COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_CURRENT_BINARY_DIR}"
COMMAND ${PROTOBUF_PROTOC_EXECUTABLE}
-I${CMAKE_CURRENT_SOURCE_DIR}
--cpp_out "${EXTRA_FLAG}${CMAKE_CURRENT_BINARY_DIR}" ${ABS_FIL}
COMMAND ${PROTOBUF_PROTOC_EXECUTABLE}
-I${CMAKE_CURRENT_SOURCE_DIR}
--cpp_out "${EXTRA_FLAG}${CMAKE_CURRENT_BINARY_DIR}" ${ABS_FIL}
DEPENDS ${ABS_FIL} protoc
COMMENT "Running C++ protocol buffer compiler on ${FIL}"
VERBATIM )
VERBATIM)
endforeach()

set_source_files_properties(${${SRCS}} ${${HDRS}} PROPERTIES GENERATED TRUE)
Expand Down
8 changes: 1 addition & 7 deletions cmake/util.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ function(target_circle_link_libraries TARGET_NAME)
if(APPLE)
set(LIBS)
set(inArchive OFF)
set(libsInArgn)

foreach(arg ${ARGN})
if(${arg} STREQUAL "ARCHIVE_START")
Expand All @@ -21,19 +20,14 @@ function(target_circle_link_libraries TARGET_NAME)
list(APPEND LIBS "-Wl,-force_load")
endif()
list(APPEND LIBS ${arg})
list(APPEND libsInArgn ${arg})
endif()
endforeach()
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
if(NOT IOS_ENABLE_BITCODE)
list(APPEND LIBS "-undefined dynamic_lookup")
endif()
endif()
list(REVERSE libsInArgn)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for curiosity, why don't we need list(REVERSE libsInArgn) anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As shown in #4892, on Mac platform I found so many libraries are linked twice, one in a given order, another in the reversed order.

-Wl,-force_load ../../gserver/libpaddle_gserver.a -Wl,-force_load ../../function/libpaddle_function.a ../../pserver/libpaddle_pserver.a ../../trainer/libpaddle_trainer_lib.a ../../pserver/libpaddle_network.a ../../math/libpaddle_math.a ../libpaddle_utils.a ../../parameter/libpaddle_parameter.a ../../../proto/libpaddle_proto.a ../../cuda/libpaddle_cuda.a ../../optimizer/libpaddle_optimizer.a 
../../../../third_party/install/gflags/lib/libgflags.a
../../../../third_party/install/glog/lib/libglog.a
/usr/local/opt/openblas/lib/libopenblas.dylib
../../../../third_party/install/protobuf/lib/libprotobuf.a
../../../../third_party/install/zlib/lib/libz.a
/usr/lib/libpython2.7.dylib
-undefined dynamic_lookup
/usr/lib/libpython2.7.dylib
../../../../third_party/install/zlib/lib/libz.a
../../../../third_party/install/protobuf/lib/libprotobuf.a
/usr/local/opt/openblas/lib/libopenblas.dylib
../../../../third_party/install/glog/lib/libglog.a
../../../../third_party/install/gflags/lib/libgflags.a
../../optimizer/libpaddle_optimizer.a ../../cuda/libpaddle_cuda.a ../../../proto/libpaddle_proto.a ../../parameter/libpaddle_parameter.a ../libpaddle_utils.a ../../math/libpaddle_math.a ../../pserver/libpaddle_network.a ../../trainer/libpaddle_trainer_lib.a ../../pserver/libpaddle_pserver.a ../../function/libpaddle_function.a ../../gserver/libpaddle_gserver.a

Here, LIBS contains all dependent libraries in given order, libsInArgn contains all the dependent in the reverse order. The reverse is done by line 32. And both LIBS and libsInArgn are linked to TARGET_NAME. I don't know why it was written like this. When I remove libsInArgn, this PR also works on Mac.

target_link_libraries(${TARGET_NAME}
${LIBS}
${libsInArgn})

target_link_libraries(${TARGET_NAME} ${LIBS})
else() # LINUX
set(LIBS)

Expand Down
2 changes: 0 additions & 2 deletions paddle/framework/lod_tensor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
#include <algorithm>
#include <iterator>

#include <glog/logging.h>

namespace paddle {
namespace framework {

Expand Down
1 change: 0 additions & 1 deletion paddle/framework/lod_tensor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include <thrust/system/cuda/experimental/pinned_allocator.h>
#endif

#include <glog/logging.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the very detail-oriented work style -- moving the inclusion of glog header file into the .cu file.

#include "paddle/framework/ddim.h"
#include "paddle/framework/tensor.h"
#include "paddle/platform/enforce.h"
Expand Down
1 change: 1 addition & 0 deletions paddle/framework/lod_tensor_test.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "paddle/framework/lod_tensor.h"
#include "paddle/platform/assert.h"

#include <glog/logging.h>
#include <gtest/gtest.h>

__global__ void test(size_t* a, int size) {
Expand Down
1 change: 0 additions & 1 deletion paddle/framework/tensor_array.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#include "paddle/framework/tensor_array.h"

#include <glog/logging.h>
#include <algorithm>
#include <limits>

Expand Down
3 changes: 3 additions & 0 deletions paddle/math/tests/test_FPException.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ int main(int argc, char** argv) {
testing::InitGoogleTest(&argc, argv);
initMain(argc, argv);

#if !defined(__arm__) && !defined(__aarch64__)
// TODO(liuyiqun) : implement the iOS version
feenableexcept(FE_INVALID | FE_DIVBYZERO | FE_OVERFLOW);
#endif
return RUN_ALL_TESTS();
}
1 change: 0 additions & 1 deletion paddle/operators/cond_op.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ limitations under the License. */

#pragma once
#include <vector>
#include "glog/logging.h"
#include "paddle/framework/ddim.h"
#include "paddle/framework/eigen.h"
#include "paddle/framework/operator.h"
Expand Down
1 change: 0 additions & 1 deletion paddle/operators/lstm_unit_op.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
limitations under the License. */

#pragma once
#include "glog/logging.h"
#include "paddle/framework/op_registry.h"

namespace paddle {
Expand Down
4 changes: 2 additions & 2 deletions paddle/platform/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
cc_library(cpu_info SRCS cpu_info.cc DEPS gflags glog)
cc_library(cpu_info SRCS cpu_info.cc DEPS gflags)
cc_test(cpu_info_test SRCS cpu_info_test.cc DEPS cpu_info)

nv_library(gpu_info SRCS gpu_info.cc DEPS gflags glog)
nv_library(gpu_info SRCS gpu_info.cc DEPS gflags)

cc_library(place SRCS place.cc)
cc_test(place_test SRCS place_test.cc DEPS place glog gflags)
Expand Down
1 change: 1 addition & 0 deletions paddle/utils/Excepts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ limitations under the License. */
#if (defined(__APPLE__) || defined(__OSX__)) && !defined(__arm__) && \
!defined(__aarch64__)

// TODO(liuyiqun): implement the iOS version
int fegetexcept(void);
int feenableexcept(unsigned int excepts);
int fedisableexcept(unsigned int excepts);
Expand Down
4 changes: 2 additions & 2 deletions paddle/utils/tests/test_StringUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ limitations under the License. */

TEST(StringUtil, to) {
ASSERT_NEAR(paddle::str::to<double>("12.45"), 12.45, 1e-5);
ASSERT_DEATH(paddle::str::to<double>("12.45x23"), ".*");
ASSERT_DEATH(paddle::str::to<int>(""), ".*");
ASSERT_DEATH_IF_SUPPORTED(paddle::str::to<double>("12.45x23"), ".*");
ASSERT_DEATH_IF_SUPPORTED(paddle::str::to<int>(""), ".*");
}