Fixes errors when linking caffe2 statically by peterjc123 · Pull Request #10765 · pytorch/pytorch

peterjc123 · 2018-08-22T02:46:59Z

Fixes #10746 and #10902.
The linking error is caused by the wrong usage of dllexport/dllimport. We don't need them if the implementation of the function is available from the external side.

orionr

If tests pass, this looks good. Thanks!

orionr · 2018-08-22T15:19:01Z

@pytorchbot retest this please

orionr · 2018-08-22T20:50:16Z

@pytorchbot retest this please

peterjc123 · 2018-08-23T04:49:03Z

@pytorchbot retest this please

peterjc123 · 2018-08-23T05:13:07Z

@pytorchbot retest this please

peterjc123 · 2018-08-26T10:59:14Z

I think the failure of CI is unrelated to this PR.

peterjc123 · 2018-08-27T08:32:31Z

@pytorchbot retest this please

xkszltl · 2018-08-28T10:27:31Z

@peterjc123

I was trying to fix this in a slightly different way and also notice some other suspicious places.

Should we also fix the linkage in its base class IdWrapper?

xkszltl · 2018-08-28T10:27:53Z

Added #10941

peterjc123 · 2018-08-28T10:31:17Z

@xkszltl Yes, that's missing. Thanks for pointing out. For the convenience of the merging of these related PRs, I've included all the required changes in this PR. So, would you please close your PR(s)?

peterjc123 · 2018-08-28T14:46:55Z

@soumith @ezyang Can we please merge this PR?

orionr · 2018-08-28T14:55:44Z

Hidden visibility (for *nix) is landing in #10752, so let's wait until that and then rebase and test. Hopefully after the above we should get better signal on Unix builds for Windows dllimport/export issues. Granted, not perfect, but better.

orionr · 2018-08-28T22:32:38Z

@pytorchbot retest this please

facebook-github-bot

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

bddppq · 2018-08-29T00:04:35Z

@pytorchbot retest this please

peterjc123 · 2018-08-29T04:04:47Z

@orionr It seems the problem comes to GCC this time. The build failure is due to this change. However, the template is already wrapped with the annotation here.

orionr · 2018-08-29T18:47:38Z

@peterjc123, correct - looks like we need to have class AT_CORE_API TypeIdentifier, since the tests reference that class, not just the methods. I'll also see if I can leave any comments on the changes here.

aten/src/ATen/core/IdWrapper.h

aten/src/ATen/core/typeid.h

peterjc123 · 2018-09-02T15:14:20Z

@orion It seems that you are right. As this post says, it seems that GCC wants the entire class exported while MSVC doesn't. So there may be two possible solutions for this issue:

Use a macro for GCC to export entire class and keep minimal with MSVC. (Not so good but easy to do)
Move the implementation into the source file to make the class-level export work with MSVC. (Seems better but may be difficult to do)

orionr · 2018-09-04T15:30:35Z

Thanks for the feedback. @peterjc123, depending on how many cases we have of this, I would recommend exploring option 2 for a day or two and then fallback to option 1. @mingzhe09088 is also seeing challenges with enabling all of Caffe2 with shared DLL builds on Windows, so it's possible that we'll hit similar issues there. So maybe option 1 is going to be required regardless.

orionr · 2018-09-04T16:33:05Z

For reference, @mingzhe09088 is doing this work at #11160

xkszltl · 2018-09-05T07:15:46Z

@peterjc123

For that stackoverflow post, I wonder if it's because implicitly defined functions, e.g. default constructor, are not exported unless we put the decorator at class level.

If that's the case, something like this might help:

class A
{
    CAFFE2_EXPORT A() = default;
};

peterjc123 · 2018-09-05T07:26:54Z

@xkszltl Thanks! I'll try this.

peterjc123 · 2018-09-05T07:50:06Z

@xkszltl Looks like it wasn't the situation that you stated. Actually, the problem is caused by the unexported symbol CAFFE_KNOWN_TYPE for GCC. But it is impossible to set the attribute visibility:default on it. See details here.

Test locally and figure out all the required changes Target comments minor changes .

aten/src/ATen/core/typeid.h

   */
  template <typename T>
-  AT_CORE_API static TypeIdentifier Id();
+  static TypeIdentifier Id();


orionr · 2018-09-07T21:49:01Z

@peterjc123 do you still have issues with building static libs on Windows with the latest master? Just checking. I feel like with the changes @Yangqing submitted we should be in good shape without these changes, but please let us know.

peterjc123 · 2018-09-08T02:45:21Z

@orionr Let me try locally first.

peterjc123 · 2018-09-08T09:38:25Z

@orionr It seems this issue is not resolved by the several Windows PRs. The error log is here:

  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) private: __cdecl caffe2::TypeMeta:
:TypeMeta(class caffe2::TypeIdentifier,unsigned __int64,void (__cdecl*)(void *,unsigned __int64),void (__cdecl*)(void c
onst *,void *,unsigned __int64),void (__cdecl*)(void *,unsigned __int64))" (__imp_??0TypeMeta@caffe2@@AEAA@VTypeIdentif
ier@1@_KP6AXPEAX1@ZP6AXPEBX21@Z3@Z) referenced in function "public: __cdecl caffe2::TensorImpl::TensorImpl<float>(class
 std::vector<__int64,class std::allocator<__int64> > const &,class std::vector<float,class std::allocator<float> > cons
t &,class caffe2::BaseContext *)" (??$?0M@TensorImpl@caffe2@@QEAA@AEBV?$vector@_JV?$allocator@_J@std@@@std@@AEBV?$vecto
r@MV?$allocator@M@std@@@3@PEAVBaseContext@1@@Z) [D:\pytorch\caffe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: unsigned __int64 const & _
_cdecl caffe2::TypeMeta::itemsize(void)const " (__imp_?itemsize@TypeMeta@caffe2@@QEBAAEB_KXZ) referenced in function "p
ublic: __cdecl caffe2::TensorImpl::TensorImpl<float>(class std::vector<__int64,class std::allocator<__int64> > const &,
class std::vector<float,class std::allocator<float> > const &,class caffe2::BaseContext *)" (??$?0M@TensorImpl@caffe2@@
QEAA@AEBV?$vector@_JV?$allocator@_J@std@@@std@@AEBV?$vector@MV?$allocator@M@std@@@3@PEAVBaseContext@1@@Z) [D:\pytorch\c
affe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void (__cdecl*__cdecl caff
e2::TypeMeta::ctor(void)const )(void *,unsigned __int64)" (__imp_?ctor@TypeMeta@caffe2@@QEBAP6AXPEAX_K@ZXZ) referenced
in function "public: void * __cdecl caffe2::TensorImpl::raw_mutable_data(class caffe2::TypeMeta const &)" (?raw_mutable
_data@TensorImpl@caffe2@@QEAAPEAXAEBVTypeMeta@2@@Z) [D:\pytorch\caffe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void (__cdecl*__cdecl caff
e2::TypeMeta::copy(void)const )(void const *,void *,unsigned __int64)" (__imp_?copy@TypeMeta@caffe2@@QEBAP6AXPEBXPEAX_K
@ZXZ) referenced in function "public: __cdecl caffe2::TensorImpl::TensorImpl<float>(class std::vector<__int64,class std
::allocator<__int64> > const &,class std::vector<float,class std::allocator<float> > const &,class caffe2::BaseContext
*)" (??$?0M@TensorImpl@caffe2@@QEAA@AEBV?$vector@_JV?$allocator@_J@std@@@std@@AEBV?$vector@MV?$allocator@M@std@@@3@PEAV
BaseContext@1@@Z) [D:\pytorch\caffe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: void (__cdecl*__cdecl caff
e2::TypeMeta::dtor(void)const )(void *,unsigned __int64)" (__imp_?dtor@TypeMeta@caffe2@@QEBAP6AXPEAX_K@ZXZ) referenced
in function "public: void * __cdecl caffe2::TensorImpl::raw_mutable_data(class caffe2::TypeMeta const &)" (?raw_mutable
_data@TensorImpl@caffe2@@QEAAPEAXAEBVTypeMeta@2@@Z) [D:\pytorch\caffe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class caffe2::TypeI
dentifier __cdecl caffe2::TypeMeta::Id<float>(void)" (__imp_??$Id@M@TypeMeta@caffe2@@SA?AVTypeIdentifier@1@XZ) referenc
ed in function "public: __cdecl caffe2::TensorImpl::TensorImpl<float>(class std::vector<__int64,class std::allocator<__
int64> > const &,class std::vector<float,class std::allocator<float> > const &,class caffe2::BaseContext *)" (??$?0M@Te
nsorImpl@caffe2@@QEAA@AEBV?$vector@_JV?$allocator@_J@std@@@std@@AEBV?$vector@MV?$allocator@M@std@@@3@PEAVBaseContext@1@
@Z) [D:\pytorch\caffe2-static-minimal\build\test-caffe2.vcxproj]
  test-caffe2.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) protected: __cdecl c10::intrusive_
ptr_target::intrusive_ptr_target(void)" (__imp_??0intrusive_ptr_target@c10@@IEAA@XZ) referenced in function "public: __
cdecl caffe2::TensorImpl::TensorImpl<float>(class std::vector<__int64,class std::allocator<__int64> > const &,class std
::vector<float,class std::allocator<float> > const &,class caffe2::BaseContext *)" (??$?0M@TensorImpl@caffe2@@QEAA@AEBV
?$vector@_JV?$allocator@_J@std@@@std@@AEBV?$vector@MV?$allocator@M@std@@@3@PEAVBaseContext@1@@Z) [D:\pytorch\caffe2-sta
tic-minimal\build\test-caffe2.vcxproj]

orionr · 2018-09-10T18:58:12Z

Looks like tests are failing to link. @mingzhe09088 is working on BUILD_PYTHON enabling at #11385 and then we're going to look at BUILD_TEST. Both are by default disabled on Windows right now, which they shouldn't be.

orionr · 2018-09-10T18:58:44Z

cc @Yangqing

peterjc123 · 2018-12-26T07:42:35Z

Closed because it's out of date.

peterjc123 requested review from apaszke, colesbury, ezyang, gchanan, soumith and zdevito as code owners August 22, 2018 02:46

orionr approved these changes Aug 22, 2018

View reviewed changes

peterjc123 force-pushed the link_fix branch from 4caef8c to 3caa210 Compare August 23, 2018 00:44

peterjc123 force-pushed the link_fix branch 2 times, most recently from e6a7b47 to 236d0b4 Compare August 25, 2018 02:49

peterjc123 force-pushed the link_fix branch from 236d0b4 to eff3e40 Compare August 27, 2018 07:05

augustofg mentioned this pull request Aug 27, 2018

LNK2019 error when linking with MSVC [Caffe2] #10902

Open

xkszltl mentioned this pull request Aug 28, 2018

[Caffe2] Template IdWrapper should not be on DLL interface. #10941

Closed

peterjc123 changed the title ~~Fixes linking errors in dispatch_test of caffe2~~ Fixes linking errors when linking caffe2 statically Aug 28, 2018

peterjc123 changed the title ~~Fixes linking errors when linking caffe2 statically~~ Fixes errors when linking caffe2 statically Aug 28, 2018

facebook-github-bot reviewed Aug 28, 2018

View reviewed changes

peterjc123 force-pushed the link_fix branch from 6451438 to db69c43 Compare August 29, 2018 01:31

orionr reviewed Aug 29, 2018

View reviewed changes

peterjc123 force-pushed the link_fix branch from 29b30c3 to c46aaae Compare September 2, 2018 06:11

orionr mentioned this pull request Sep 4, 2018

Build Caffe2 shared library on Windows #11160

Closed

peterjc123 force-pushed the link_fix branch from 5fa058e to fc9f880 Compare September 7, 2018 03:16

peterjc123 added 2 commits September 7, 2018 12:21

Fixes linking errors in dispatch_test of caffe2

718b636

Test locally and figure out all the required changes Target comments minor changes .

.

b11221e

peterjc123 force-pushed the link_fix branch from fc9f880 to b11221e Compare September 7, 2018 04:22

.

1b64cc6

peterjc123 commented Sep 7, 2018

View reviewed changes

aten/src/ATen/core/typeid.h

*/

template <typename T>

AT_CORE_API static TypeIdentifier Id();

static TypeIdentifier Id();

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

peterjc123 closed this Dec 26, 2018

peterjc123 deleted the link_fix branch April 5, 2019 14:59

ezyang added the open source label Jun 24, 2019

Conversation

peterjc123 commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orionr left a comment

Choose a reason for hiding this comment

Uh oh!

orionr commented Aug 22, 2018

Uh oh!

orionr commented Aug 22, 2018

Uh oh!

peterjc123 commented Aug 23, 2018

Uh oh!

peterjc123 commented Aug 23, 2018

Uh oh!

peterjc123 commented Aug 26, 2018

Uh oh!

peterjc123 commented Aug 27, 2018

Uh oh!

xkszltl commented Aug 28, 2018

Uh oh!

xkszltl commented Aug 28, 2018

Uh oh!

peterjc123 commented Aug 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterjc123 commented Aug 28, 2018

Uh oh!

orionr commented Aug 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orionr commented Aug 28, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

bddppq commented Aug 29, 2018

Uh oh!

peterjc123 commented Aug 29, 2018

Uh oh!

orionr commented Aug 29, 2018

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

peterjc123 commented Sep 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orionr commented Sep 4, 2018

Uh oh!

orionr commented Sep 4, 2018

Uh oh!

xkszltl commented Sep 5, 2018

Uh oh!

peterjc123 commented Sep 5, 2018

Uh oh!

peterjc123 commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

orionr commented Sep 7, 2018

Uh oh!

peterjc123 commented Sep 8, 2018

Uh oh!

peterjc123 commented Sep 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

peterjc123 commented Aug 22, 2018 •

edited

Loading

peterjc123 commented Aug 28, 2018 •

edited

Loading

orionr commented Aug 28, 2018 •

edited

Loading

peterjc123 commented Sep 2, 2018 •

edited

Loading

peterjc123 commented Sep 5, 2018 •

edited

Loading

peterjc123 commented Sep 8, 2018 •

edited

Loading