Skip to content

Fix missing symbol linker error when using libtorch generated on windows :#13672

Closed
lara-hdr wants to merge 1 commit intopytorch:masterfrom
lara-hdr:windows_export_symbols
Closed

Fix missing symbol linker error when using libtorch generated on windows :#13672
lara-hdr wants to merge 1 commit intopytorch:masterfrom
lara-hdr:windows_export_symbols

Conversation

@lara-hdr
Copy link
Contributor

@lara-hdr lara-hdr commented Nov 7, 2018

Libtorch is missing some symbols when generated on windows, causing linker errors when using it.

It seems like there were some issues in the past with enabling CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS to export all symbols during the build.
(See the link below :
- Enabling CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS : #3617
- Disabling CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS : #9092 and #9693 )

So enabling CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS is not an option. But some symbols are still missing for Libtorch to be working.
We added some functions to TORCH_API in this PR, but we might be missing some.
(We also tried adding the whole structure Method (struct TORCH_API Method { ... }) instead of adding the functions separately, but the build fails with a "one or more multiply defined symbols found" error)

Do you have any recommendations on how to detect functions that should/shouldn't be in TORCH_API, so the build is successful and the generated Libtorch has all the required exported symbols?

I also attached toch_exports_missing.txt, which contains the symbols that are exported with the CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS flag enabled but not in the current Libtorch version.
( by generating the output for both torch.dll libraries with "dumpbin /EXPORTS torch.dll" and comparing both outputs and generating the difference)
So any symbol that could be missing from Libtorch should be in this list, but the list has more than 8000 symbols, and I am not sure which ones require to be exported and added to TORCH_API.

This PR currently exports the missing symbols for torch::jit::script::Method that appears in the attached list (in the exception of defaultSchemaFor, and emit_call_to that cause a "multiply defined symbols" error).

torch_exports_missing.txt

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 7, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@soumith has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@lara-hdr
Copy link
Contributor Author

lara-hdr commented Nov 14, 2018

Hey @soumith this has been approved and open for a couple of days, is it possible to merge it to master? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

oncall: jit Add this issue/PR to JIT oncall triage queue open source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants