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

CMake cross-generator fixes #2790

Merged
merged 12 commits into from
Jan 14, 2020
Merged

CMake cross-generator fixes #2790

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2db77a6
Fix compilation w/ non-VS CMake generators
tiagoshibata Dec 18, 2019
8e3aa0c
Fix custom WINMD target in Ninja
tiagoshibata Jan 6, 2020
a3591f8
Remove usage of msbuild .targets file
tiagoshibata Jan 7, 2020
177aee0
Fix linking using DML in Ninja
tiagoshibata Jan 7, 2020
1547006
Automate SDK kit version choice
tiagoshibata Jan 7, 2020
42f701c
Cleanup DML package install
tiagoshibata Jan 7, 2020
125667d
Fix SDK version detection
tiagoshibata Jan 7, 2020
5c39112
Fix comment
tiagoshibata Jan 7, 2020
d856b56
Revert unittest linkage changes
tiagoshibata Jan 7, 2020
1809c22
Fix latest SDK detection
tiagoshibata Jan 7, 2020
76b3259
Don't link to non-uapcore libraries
tiagoshibata Jan 10, 2020
53ba310
Remove MessageBoxA reference and unused link libs
tiagoshibata Jan 11, 2020
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
8 changes: 3 additions & 5 deletions cmake/external/dml.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,17 @@ if (NOT onnxruntime_USE_CUSTOM_DIRECTML)
set(NUGET_CONFIG ${PROJECT_SOURCE_DIR}/../NuGet.config)
set(PACKAGES_CONFIG ${PROJECT_SOURCE_DIR}/../packages.config)
set(PACKAGES_DIR ${CMAKE_CURRENT_BINARY_DIR}/packages)
set(DML_PACKAGE_DIR ${PACKAGES_DIR}/DirectML.0.0.1)

# Restore nuget packages, which will pull down the DirectML redist package
add_custom_command(
OUTPUT restore_packages.stamp
OUTPUT ${DML_PACKAGE_DIR}/bin/x64/DirectML.lib ${DML_PACKAGE_DIR}/bin/x86/DirectML.lib
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ninja failed with ninja: error: 'packages/DirectML.0.0.1/bin/x64/DirectML.lib', needed by 'onnxruntime.lib', missing and no known rule to make it, given that it didn't know how to get DirectML.lib which was a dep of ORT.

With msbuild it just happened to not check if it could satisfy all dependencies first and happened to run the RESTORE_PACKAGES target before building ORT.

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to pull down both x64 and x86 all the time (is that what this does?)? also, what about arm/arm64?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I'm declaring that this command will output x64/DirectML.lib and x86/DirectML.lib ; if any target depends on either of those, the package will be downloaded using Nuget. There's a single nuget package with both, no option to pull them separatly.

ARM is not supported, I was talking to Sheil about it last week. DML distributes a redist package with x86 and x64 DML, but this public redist package doesn't have ARM (seems like they didn't have a need for it and didn't want to build and maintain it if no customers are using it. We were not sure if that's the only reason, we can talk to the DML folks if we need an ARM redist package)

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the intention was to have onnxruntime depend on onnxruntime_EXTERNAL_DEPENDENCIES, which implicitly includes the DML libs. This change looks fine too, and it's more explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right, but it doesn't work with ninja. ORT links with DirectML.lib, which doesn't exist when doing a clean build because the nuget package isn't downloaded yet, and there is no rule on how to generate DirectML.lib, so it says error: 'packages/DirectML.0.0.1/bin/x64/DirectML.lib', needed by 'onnxruntime.lib', missing and no known rule to make it.

Depending on onnxruntime_EXTERNAL_DEPENDENCIES will eventually pull DirectML.lib, but there's no way the build system knows about it before pulling and unpacking the package. It seems like msbuild doesn't validate that it can actually resolve everything before starting the build, so it worked before.

Copy link
Contributor

Choose a reason for hiding this comment

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

When I originally set this up, cmake actually had a similar problem. It barfed when trying to target_link_libraries the DirectML.targets file, because the nuget package hadn't been downloaded yet (and therefore the targets file didn't exist).

This was solved by adding an explicit step into build.py which invokes the RESTORE_PACKAGES step prior to running the "real" build. This ensures that when the build starts, the nuget package is already present.

Why doesn't that solution work in this case?

Copy link
Contributor Author

@tiagoshibata tiagoshibata Jan 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Oh sure, that also works. I didn't realize RESTORE_PACKAGES was explicitly called in build.py.

I was running the ninja build "by hand" (calling cmake followed by cmake --build or configuring the cmake project in VS directly).

Is there a reason not to fix it the way I did?

DEPENDS ${PACKAGES_CONFIG} ${NUGET_CONFIG}
COMMAND ${CMAKE_CURRENT_BINARY_DIR}/nuget/src/nuget restore ${PACKAGES_CONFIG} -PackagesDirectory ${PACKAGES_DIR} -ConfigFile ${NUGET_CONFIG}
COMMAND ${CMAKE_COMMAND} -E touch restore_packages.stamp
VERBATIM)

add_custom_target(RESTORE_PACKAGES ALL DEPENDS restore_packages.stamp)
add_custom_target(RESTORE_PACKAGES ALL DEPENDS ${DML_PACKAGE_DIR}/bin/x64/DirectML.lib ${DML_PACKAGE_DIR}/bin/x86/DirectML.lib)
add_dependencies(RESTORE_PACKAGES nuget)

list(APPEND onnxruntime_EXTERNAL_DEPENDENCIES RESTORE_PACKAGES)
else()
include_directories(${dml_INCLUDE_DIR})
endif()
16 changes: 16 additions & 0 deletions cmake/onnxruntime_common.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ else()
endif()
endif()

if(CMAKE_GENERATOR_PLATFORM)
# Multi-platform generator
set(onnxruntime_target_platform ${CMAKE_GENERATOR_PLATFORM})
else()
set(onnxruntime_target_platform ${CMAKE_SYSTEM_PROCESSOR})
endif()
if(onnxruntime_target_platform STREQUAL "ARM64")
set(onnxruntime_target_platform "ARM64")
elseif(onnxruntime_target_platform STREQUAL "ARM" OR CMAKE_GENERATOR MATCHES "ARM")
set(onnxruntime_target_platform "ARM")
elseif(onnxruntime_target_platform STREQUAL "x64" OR onnxruntime_target_platform STREQUAL "x86_64" OR onnxruntime_target_platform STREQUAL "AMD64" OR CMAKE_GENERATOR MATCHES "Win64")
set(onnxruntime_target_platform "x64")
elseif(onnxruntime_target_platform STREQUAL "x86" OR onnxruntime_target_platform STREQUAL "i386" OR onnxruntime_target_platform STREQUAL "i686")
set(onnxruntime_target_platform "x86")
endif()

Comment on lines +53 to +62
Copy link
Contributor Author

@tiagoshibata tiagoshibata Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

A bit ugly, but we must take into consideration possible values of CMAKE_GENERATOR_PLATFORM (platform in multi-platform generators), CMAKE_SYSTEM_PROCESSOR (target CPU in single platform generators) and CMAKE_GENERATOR, and CMAKE_SYSTEM_PROCESSOR can take a range of values (from docs: On systems that support uname, this variable is set to the output of uname -p. On Windows it is set to the value of the environment variable PROCESSOR_ARCHITECTURE.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be checking CMAKE_GENERATOR_PLATFORM MATCHES "ARM" (line 53, 57), not CMAKE_GENERATOR? CMAKE_GENERATOR will be something like "Visual Studio 15 2017" or "Ninja", not an architecture.

Copy link
Contributor Author

@tiagoshibata tiagoshibata Jan 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

For older versions of the VS CMake generator, the platform is part of the generator string. See cmake --help:

Generators

The following generators are available on this platform (* marks default):
* Visual Studio 16 2019        = Generates Visual Studio 2019 project files.
                                 Use -A option to specify architecture.
  Visual Studio 15 2017 [arch] = Generates Visual Studio 2017 project files.
                                 Optional [arch] can be "Win64" or "ARM".
  Visual Studio 14 2015 [arch] = Generates Visual Studio 2015 project files.
                                 Optional [arch] can be "Win64" or "ARM".
  Visual Studio 12 2013 [arch] = Generates Visual Studio 2013 project files.
                                 Optional [arch] can be "Win64" or "ARM".
  Visual Studio 11 2012 [arch] = Generates Visual Studio 2012 project files.
                                 Optional [arch] can be "Win64" or "ARM".

file(GLOB onnxruntime_common_src CONFIGURE_DEPENDS
${onnxruntime_common_src_patterns}
)
Expand Down
6 changes: 3 additions & 3 deletions cmake/onnxruntime_mlas.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ set(mlas_common_srcs
)

if(MSVC)
if(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM64")
if(onnxruntime_target_platform STREQUAL "ARM64")
set(asm_filename ${ONNXRUNTIME_ROOT}/core/mlas/lib/arm64/SgemmKernelNeon.asm)
set(pre_filename ${CMAKE_CURRENT_BINARY_DIR}/SgemmKernelNeon.i)
set(obj_filename ${CMAKE_CURRENT_BINARY_DIR}/SgemmKernelNeon.obj)
Expand All @@ -38,11 +38,11 @@ if(MSVC)
armasm64.exe ${ARMASM_FLAGS} ${pre_filename} ${obj_filename}
)
set(mlas_platform_srcs ${obj_filename})
elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM" OR CMAKE_GENERATOR MATCHES "ARM")
elseif(onnxruntime_target_platform STREQUAL "ARM")
set(mlas_platform_srcs
${ONNXRUNTIME_ROOT}/core/mlas/lib/arm/sgemmc.cpp
)
elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "x64" OR CMAKE_GENERATOR MATCHES "Win64")
elseif(onnxruntime_target_platform STREQUAL "x64")
enable_language(ASM_MASM)

set(mlas_platform_srcs
Expand Down
35 changes: 25 additions & 10 deletions cmake/onnxruntime_providers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ if (onnxruntime_USE_TENSORRT)
if ( CMAKE_COMPILER_IS_GNUCC )
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-parameter -Wno-missing-field-initializers")
endif()
set(CXX_VERSION_DEFINED TRUE)
set(CXX_VERSION_DEFINED TRUE)
add_subdirectory(${ONNXRUNTIME_ROOT}/../cmake/external/onnx-tensorrt)
set(CMAKE_CXX_FLAGS ${OLD_CMAKE_CXX_FLAGS})
if (WIN32)
Expand Down Expand Up @@ -303,7 +303,7 @@ if (onnxruntime_USE_OPENVINO)
if(WIN32)
set(OPENVINO_LIB_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/lib/intel64/Release)
set(OPENVINO_TBB_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/lib/intel64/Release)
set(OPENVINO_MKL_TINY_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/bin/intel64/Release)
set(OPENVINO_MKL_TINY_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/bin/intel64/Release)
else()
set(OPENVINO_LIB_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/lib/intel64/)
set(OPENVINO_TBB_DIR $ENV{INTEL_OPENVINO_DIR}/deployment_tools/inference_engine/external/tbb/lib)
Expand All @@ -327,9 +327,9 @@ if (onnxruntime_USE_OPENVINO)
else()
target_include_directories(onnxruntime_providers_openvino SYSTEM PUBLIC ${ONNXRUNTIME_ROOT} ${eigen_INCLUDE_DIRS} ${OPENVINO_INCLUDE_DIR} ${OPENVINO_EXTENSIONS_DIR} ${OPENVINO_LIB_DIR} ${OPENVINO_TBB_INCLUDE_DIR} ${PYTHON_INCLUDE_DIRS})
endif()
if (WIN32)
string(REPLACE "include" "libs" PYTHON_LIB ${PYTHON_INCLUDE_DIRS})

if (WIN32)
string(REPLACE "include" "libs" PYTHON_LIB ${PYTHON_INCLUDE_DIRS})
find_package(InferenceEngine 2.1 REQUIRED)
set(PYTHON_LIBRARIES ${PYTHON_LIB})
set(OPENVINO_CPU_EXTENSION_DIR ${onnxruntime_BINARY_DIR}/ie_cpu_extension/${CMAKE_BUILD_TYPE})
Expand Down Expand Up @@ -430,22 +430,37 @@ if (onnxruntime_USE_DML)
onnxruntime_add_include_to_target(onnxruntime_providers_dml onnxruntime_common onnxruntime_framework onnx onnx_proto protobuf::libprotobuf)
add_dependencies(onnxruntime_providers_dml ${onnxruntime_EXTERNAL_DEPENDENCIES})
target_include_directories(onnxruntime_providers_dml PRIVATE ${ONNXRUNTIME_ROOT} ${ONNXRUNTIME_ROOT}/../cmake/external/wil/include)

target_link_libraries(onnxruntime_providers_dml ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_link_libraries(onnxruntime_providers_dml d3d12.lib dxgi.lib)

if(NOT onnxruntime_target_platform STREQUAL "x86" AND NOT onnxruntime_target_platform STREQUAL "x64")
message(FATAL_ERROR "Target platform ${onnxruntime_target_platform} is not supported by DML")
endif()
foreach(file "DirectML.dll" "DirectML.pdb" "DirectML.Debug.dll" "DirectML.Debug.pdb")
add_custom_command(TARGET onnxruntime_providers_dml
POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_if_different
"${DML_PACKAGE_DIR}/bin/${onnxruntime_target_platform}/${file}" $<TARGET_FILE_DIR:onnxruntime_providers_dml>)
endforeach()

function(target_add_dml target)
target_link_libraries(${target} PRIVATE "${DML_PACKAGE_DIR}/bin/${onnxruntime_target_platform}/DirectML.lib")
target_include_directories(${target} PRIVATE "${DML_PACKAGE_DIR}/include")
endfunction()

target_add_dml(onnxruntime_providers_dml)
target_link_libraries(onnxruntime_providers_dml PRIVATE d3d12.lib dxgi.lib delayimp.lib)
list(APPEND ONNXRUNTIME_LINKER_FLAGS "/DELAYLOAD:DirectML.dll /DELAYLOAD:d3d12.dll /DELAYLOAD:dxgi.dll")

# The DML EP requires C++17
set_target_properties(onnxruntime_providers_dml PROPERTIES CXX_STANDARD 17)
set_target_properties(onnxruntime_providers_dml PROPERTIES CXX_STANDARD_REQUIRED ON)

target_compile_definitions(onnxruntime_providers_dml PRIVATE ONNX_NAMESPACE=onnx ONNX_ML LOTUS_LOG_THRESHOLD=2 LOTUS_ENABLE_STDERR_LOGGING PLATFORM_WINDOWS)
target_compile_definitions(onnxruntime_providers_dml PRIVATE UNICODE _UNICODE NOMINMAX)
if (MSVC)
target_compile_definitions(onnxruntime_providers_dml PRIVATE _SILENCE_CXX17_ITERATOR_BASE_CLASS_DEPRECATION_WARNING)
target_compile_options(onnxruntime_providers_dml PRIVATE "/W3")
endif()

install(DIRECTORY ${PROJECT_SOURCE_DIR}/../include/onnxruntime/core/providers/dml DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/onnxruntime/core/providers)

set_target_properties(onnxruntime_providers_dml PROPERTIES LINKER_LANGUAGE CXX)
Expand Down
12 changes: 6 additions & 6 deletions cmake/onnxruntime_unittests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function(AddTest)
endif()
if (onnxruntime_ENABLE_LANGUAGE_INTEROP_OPS AND onnxruntime_ENABLE_PYTHON)
target_compile_definitions(${_UT_TARGET} PRIVATE ENABLE_LANGUAGE_INTEROP_OPS)
endif()
endif()
if (WIN32)
if (onnxruntime_USE_CUDA)
# disable a warning from the CUDA headers about unreferenced local functions
Expand Down Expand Up @@ -318,7 +318,7 @@ if (onnxruntime_USE_DNNL)
target_compile_definitions(onnxruntime_test_utils_for_framework PUBLIC USE_DNNL=1)
endif()
if (onnxruntime_USE_DML)
target_link_libraries(onnxruntime_test_utils_for_framework PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_add_dml(onnxruntime_test_utils_for_framework)
endif()
add_dependencies(onnxruntime_test_utils_for_framework ${onnxruntime_EXTERNAL_DEPENDENCIES})
target_include_directories(onnxruntime_test_utils_for_framework PUBLIC "${TEST_SRC_DIR}/util/include" PRIVATE ${eigen_INCLUDE_DIRS} ${ONNXRUNTIME_ROOT})
Expand All @@ -336,7 +336,7 @@ if (onnxruntime_USE_DNNL)
target_compile_definitions(onnxruntime_test_utils PUBLIC USE_DNNL=1)
endif()
if (onnxruntime_USE_DML)
target_link_libraries(onnxruntime_test_utils PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_add_dml(onnxruntime_test_utils)
endif()
add_dependencies(onnxruntime_test_utils ${onnxruntime_EXTERNAL_DEPENDENCIES})
target_include_directories(onnxruntime_test_utils PUBLIC "${TEST_SRC_DIR}/util/include" PRIVATE ${eigen_INCLUDE_DIRS} ${ONNXRUNTIME_ROOT})
Expand Down Expand Up @@ -722,7 +722,7 @@ if (onnxruntime_BUILD_SERVER)
set_source_files_properties("${TEST_SRC_DIR}/server/unit_tests/executor_test.cc" PROPERTIES COMPILE_FLAGS -Wno-unused-parameter)
endif()
endif()

add_library(onnxruntime_test_utils_for_server ${onnxruntime_test_server_src})
onnxruntime_add_include_to_target(onnxruntime_test_utils_for_server onnxruntime_test_utils_for_framework gtest gmock onnx onnx_proto server_proto server_grpc_proto)
add_dependencies(onnxruntime_test_utils_for_server onnxruntime_server_lib onnxruntime_server_http_core_lib Boost ${onnxruntime_EXTERNAL_DEPENDENCIES})
Expand Down Expand Up @@ -751,13 +751,13 @@ if (onnxruntime_BUILD_SERVER)
LANGUAGE python
TARGET onnxruntime_server_tests
OUT_VAR server_test_py)

set(grpc_py "${CMAKE_CURRENT_BINARY_DIR}/prediction_service_pb2_grpc.py")

add_custom_command(
TARGET onnxruntime_server_tests
COMMAND $<TARGET_FILE:protobuf::protoc>
ARGS
ARGS
--grpc_out "${CMAKE_CURRENT_BINARY_DIR}"
--plugin=protoc-gen-grpc="${_GRPC_PY_PLUGIN_EXECUTABLE}"
-I ${grpc_proto_path}
Expand Down
15 changes: 7 additions & 8 deletions cmake/precompiled_header.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
# header name as input. The function will generate a .cpp file that includes the header and is used
# to generate the precompiled header; this source file is added to the target's sources.
function(target_precompiled_header target_name header_name)
if (MSVC)

if (MSVC AND CMAKE_VS_PLATFORM_TOOLSET)
Copy link
Contributor Author

@tiagoshibata tiagoshibata Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

PCHs should work under ninja, but the way it's built is currently broken (it writes a file named ${header_base_name}.cpp. Problem is that there are multiple pchs called pch.h in WinML, which when built in parallel leads to Ninja giving permission denied or internal compiler errors when trying to write pch.cpp at the same time). So this was already broken, but happened to work under msbuild. We should fix the repo to remove headers with duplicate names.

Copy link
Contributor

Choose a reason for hiding this comment

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

curious - why can't headers with the same name exist in different folders? that seems like it should be supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be supported, it's just that right now it writes pch.cpp to ${CMAKE_CURRENT_BINARY_DIR}/${header_base_name}.cpp (at this line) and compiles it, so two headers with the same base name go to the same cpp file. If we want to support it we'd have to change precompiled_header.cmake to write to unique file names or different directories.

# The input precompiled header source (i.e. the '.h' file used for the precompiled header).
set(pch_header_path ${header_name})
get_filename_component(header_base_name ${header_name} NAME_WE)
Expand All @@ -14,14 +13,14 @@ function(target_precompiled_header target_name header_name)
set(pch_source_content "// THIS FILE IS GENERATED BY CMAKE\n#include \"${pch_header_path}\"")
file(WRITE ${pch_source_path} ${pch_source_content})
set_source_files_properties(${pch_source_path} PROPERTIES COMPILE_FLAGS "/Yc${pch_header_path}")

# The target's C++ sources use the precompiled header (/Yu). Source-level properties will
# take precedence over target-level properties, so this will not change the generated source
# take precedence over target-level properties, so this will not change the generated source
# file's property to create the precompiled header (/Yc).
target_compile_options(${target_name} PRIVATE $<$<COMPILE_LANGUAGE:CXX>:/Yu${header_name}>)

# Append generated precompiled source to target's sources.
target_sources(${target_name} PRIVATE ${pch_source_path})
endif(MSVC)
endfunction()

endif()
endfunction()
11 changes: 7 additions & 4 deletions cmake/winml.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ list(APPEND winml_adapter_files
${winml_adapter_dir}/WinMLAdapter.cpp
${winml_adapter_dir}/WinMLAdapter.h
${winml_adapter_dir}/ZeroCopyInputStreamWrapper.cpp
${winml_adapter_dir}/ZeroCopyInputStreamWrapper.h
${winml_adapter_dir}/ZeroCopyInputStreamWrapper.h
)

if (onnxruntime_USE_DML)
Expand All @@ -159,6 +159,7 @@ add_dependencies(winml_adapter ${onnxruntime_EXTERNAL_DEPENDENCIES})
target_precompiled_header(winml_adapter pch.h)

# Includes
target_include_directories(winml_adapter PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) # windows machine learning generated component headers
target_include_directories(winml_adapter PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml_api) # windows machine learning generated component headers
target_include_directories(winml_adapter PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml_api/comp_generated) # windows machine learning generated component headers
target_include_directories(winml_adapter PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml/sdk/cppwinrt/include) # sdk cppwinrt headers
Expand All @@ -181,7 +182,7 @@ add_dependencies(winml_adapter winml_api_native_internal)
# Link libraries
target_link_libraries(winml_adapter PRIVATE wil)
if (onnxruntime_USE_DML)
target_link_libraries(winml_adapter PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_add_dml(winml_adapter)
endif(onnxruntime_USE_DML)

# add it to the onnxruntime shared library
Expand Down Expand Up @@ -230,6 +231,7 @@ target_compile_definitions(winml_lib_image PRIVATE _SCL_SECURE_NO_WARNINGS)
target_precompiled_header(winml_lib_image pch.h)

# Includes
target_include_directories(winml_lib_image PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) # windows machine learning generated component headers
target_include_directories(winml_lib_image PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml_api) # windows machine learning generated component headers
target_include_directories(winml_lib_image PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml_api/comp_generated) # windows machine learning generated component headers
target_include_directories(winml_lib_image PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/winml/sdk/cppwinrt/include) # sdk cppwinrt headers
Expand Down Expand Up @@ -258,7 +260,7 @@ add_dependencies(winml_lib_image winml_api_native_internal)
# Link libraries
target_link_libraries(winml_lib_image PRIVATE wil)
if (onnxruntime_USE_DML)
target_link_libraries(winml_lib_image PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_add_dml(winml_lib_image)
endif(onnxruntime_USE_DML)


Expand Down Expand Up @@ -360,7 +362,7 @@ add_dependencies(winml_lib_api winml_api_native_internal)
# Link libraries
target_link_libraries(winml_lib_api PRIVATE wil)
if (onnxruntime_USE_DML)
target_link_libraries(winml_lib_api PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/packages/DirectML.0.0.1/build/DirectML.targets)
target_add_dml(winml_lib_api)
endif(onnxruntime_USE_DML)


Expand Down Expand Up @@ -472,6 +474,7 @@ target_link_libraries(winml_dll PRIVATE winml_lib_api)
target_link_libraries(winml_dll PRIVATE winml_lib_image)
target_link_libraries(winml_dll PRIVATE winml_lib_telemetry)
target_link_libraries(winml_dll PRIVATE onecoreuap_apiset.lib)
target_link_libraries(winml_dll PRIVATE delayimp.lib)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor Author

@tiagoshibata tiagoshibata Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Linking was failing due to missing a implementation of __delayLoadHelper2. According to the docs, delayimp.lib must be linked when delay loading libraries or a custom implementation of __delayLoadHelper2 must be provided: If you do not plan to use your own version of a helper function, you must also link your program with delayimp.lib (for desktop applications) or dloadhelper.lib (for store apps)

I don't know why it worked with msbuild, maybe it added delayimp.lib automatically?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you make sure this doesn't add dependencies on desktop-only binaries? We need to build for onecore-based SKUs as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll run msbuild and look at its linker command line to find out why it works there and not with ninja.

If I find out that delayimp is really needed, how can I test in other platforms to make sure I didn't berak non-desktop builds?

target_link_libraries(winml_dll PRIVATE ${DBGHELP})

# 1 of 3 projects that fail in link with 'failed to do memory mapped file I/O' (Only release)
Expand Down
Loading