DPC++ link error workaround. by yas-sim · Pull Request #4192 · openvinotoolkit/openvino

yas-sim · 2021-02-05T06:11:10Z

OpenVINO C++ program failed to link when DPC++ compiler is used.
'make_shared_blob' causes 'unresolved external symbol' error on linking.
Commented out some clang specific directives to workaround the issue in "ie_blob.h".

OpenVINO C++ program failed to link when DPC++ compiler is used. 'make_shared_blob' causes 'unresolved external symbol' error on linking. Commented out some __clang__ specific directives to workaround the issue in "ie_blob.h".

ilyachur · 2021-02-05T07:18:54Z

inference-engine/include/ie_blob.h

-#ifdef __clang__
-    virtual ~TBlob();
-#else
+//#ifdef __clang__


We shouldn't have a commented code in OpenVINO headers.

BTW I am not sure that we won't have issues with RTTI on macOS.

may be we need to use different approach: export not only for clang, but for gcc as well. In this case binaries compiled with gcc can be used with clang after

CC @dkurt @IRDonch

I think my fix is a kind of temporary workaround and there might be more better way to solve the issue. I don't stick to my solution.
BTW, I attached the source code and cmake file which I compiled with DPC++ compiler for issue reproduction, just for in case.
openvino-cnn.zip

This fix will break Android setup. I'd like to recommend to keep just a definition virtual ~TBlob(); but remove conditional compilation for TBlob's destructor at

openvino/inference-engine/src/inference_engine/ie_rtti.cpp

Lines 105 to 109 in e1a7f7e

#ifdef __clang__

template <typename T, typename U>

TBlob<T, U>::~TBlob() {

free();

}

I tried to leave 'virtual ~TBlob();' but the 'unresolved reference' error occur again.
Compile successfully done with 'virtual ~TBlob() {};' on both DPC++ and MSVC2019 compilers, so I committed this change and pushed this as fix #2.

@dkurt am I right that it does not solve the issue CVS-41513 which is exactly the same as this one, but for different compiler (compiled with gcc, used with clang). Can we have generic solution?

@ilya-lavrenov, I would like to remove conditional compilation at all.

1. Removed type-by-type template class definition for __clang__. 2. Modified TBlob() destructor. The 'unresolved reference' error occur again if I left 'virtual ~TBlob();' only. It seems it needs to be 'virtual ~TBlob() {};'.

dkurt · 2021-02-08T12:16:02Z

inference-engine/include/ie_blob.h

     */
 #ifdef __clang__
-    virtual ~TBlob();
+    virtual ~TBlob() {};


That breaks RTTI for Android (see https://android.googlesource.com/platform/ndk/+/master/docs/user/common_problems.md#rtti_exceptions-not-working-across-library-boundaries). So keep implementation in the .cpp file.

I think the root cause of this issue is, different type of compiler is used to build the library (IE) and the user program. In this case, IE libraries will be built by MSVC and the user code is built by a Clang based DPC++. I can add something like 'InferenceEngine::TBlob ~TBlob() {};' into an IE CPP source file with clang conditional directive, the code won't be compiled because MSVC will be used for building IE.
Instead, I added conditional directive in ie_blob.h to check if the compiler is DPC++ or not. It requires user code to include <CL/sycl.hpp> so that the __SYCL_COMPILER_VERSION macro is defined.
I have pushed this fix. Please take a look.

Uses '__SYCL_COMPILER_VERSION' predefined macro to check if the compiler is a DPC++ or not. Added conditional directive to switch code based of the detected compiler. NOTE: User program must include <CL/sycl.hpp>, or the '__SYCL_COMPILER_VERSION' macro won't be defined and this fix won't take effect.

inference-engine/include/ie_blob.h

Changed from #ifdef to #if + logical formulas.

inference-engine/include/ie_blob.h

Added compiler check logic in src/ie_rtti.cpp

dkurt · 2021-02-09T13:00:00Z

inference-engine/src/inference_engine/ie_rtti.cpp

 template struct InferenceEngine::Parameter::RealData<std::tuple<unsigned int, unsigned int, unsigned int>>;
 template struct InferenceEngine::Parameter::RealData<InferenceEngine::Blob::Ptr>;
-#endif  // __clang__
+#endif  // __clang__ && !__SYCL_COMPILER_VERSION


If you need Parameter as well as TBlob so add the same macro to https://github.com/openvinotoolkit/openvino/blob/master/inference-engine/include/ie_parameter.hpp

Have you tested the compilation?

Oops. I could build 2021.2 tag version but cmake failed with the current master branch. So, I can't compiled the code now. Sorry to trouble you. Do you have any suggestion for following cmake error (Windows) ?

CMake Error at inference-engine/thirdparty/CMakeLists.txt:98 (add_library):
add_library cannot create ALIAS target "mkldnn" because target "dnnl" does
not already exist.

I have checked the cmake file at at inference-engine/thirdparty/CMakeLists.txt:98 but no idea how to resolve the issue.
98 add_library(mkldnn ALIAS dnnl)

I have downloaded MKL-DNN with 'prepare_mkl.bat'.

Also, I have installed oneAPI base kit (w/DNNL) and DNNLROOT environment variable is properly set (c:\Program Files (x86)\Intel\oneAPI\dnnl\2021.1.1\env..\cpu_tbb).

I have tried to build the master branch without CPU plugin (MKL plugin) by adding -DENABLE_MKL_DNN=OFF to the cmake options, and I could build IE without any error (Windows 10).

Do I still need to add the same macro to ie_parameter.hpp for Android?

I've added the same compiler check macro to ie_parameter.cpp and pushed it as the fix #6.
(And, I have checked the code can be successfully compiled (wo/MKLDNNplugin, though) on Win10 environment)

Added compiler check macro for DPC++ to ie_parameter.cpp as well.

dkurt · 2021-02-12T05:37:14Z

Seems like internal CI has not started.

ilyachur · 2021-02-12T07:08:32Z

Seems like internal CI has not started.

I asked to run it manually.

yas-sim · 2021-02-13T00:25:14Z

It looks CI has failed. Is there anything I can do?

ilyachur · 2021-02-13T04:16:13Z

It looks CI has failed. Is there anything I can do?

I re-run the failed step.

* fix ss * successfully converted * successfully run moved infer and normalizer unit-tests * successfully rewritten StridedSlice infer unittests * int64 array * Successfully converter crash-when-loading, xj_feauture and toy nets (cherry-picked maxpoolV4 and tf_broadcast_ext) * successfully moved PermuteAttrs to general mechanism * successfully converted xj_feauture and crash when loading with the new rewritten SS infer * fixed get_shape_from_slice and moved to common utils * fixed extending masks and some other * some refactoring * fixed extending masks in extractor, fixed licence year and some other code clearing * corrected a couple of unittests * fox permute for 5 rank slice and 4 rank inputs/ * WIP * Added comments * fixed StridedSlice in ProposalMutation.py * rechecked shape_infer unittests added some new cases * added shape_infer unit-tests after StridedSliceNormalizer pass and Permute unit-tests * corrected unittests * Applied review comments * general permutations for inputs implemented, corrected ellipsis unrolling when shrink_axis is at the beginning, some other corrections * removed code duplication in infer and normalizer, moved 'slices' attr normalizing to StridedSliceNormalizer.py * removed some code duplication and other minor improvements * Added tests * minor corrections * [GNA] Support 1d tensors (openvinotoolkit#4270) * [OpenVINO Scripts] Updated with RHEL8 (openvinotoolkit#4296) * Updated install_NEO_OCL_driver.sh & install_openvino_dependencies.sh with rhel8 case * install_NEO_OCL_driver: Fixed conditional * script: Updated with actual revisions * [CPU] MKLDNN NCHW pooling primitive performance fix. (openvinotoolkit#4235) * [IE][VPU]: Check memory capacity after finding the corresponding device (openvinotoolkit#4314) This issue relates to multi-device mode. While we are trying to allocate a graph on one of the devices we should check memory capacity only for the corresponding device, not for the last opened as far devices may have different memory capacity or the latest opened device may have still not sent its attributes (including memory capacity) * Azure CI: Enable IB initiators as helpers * Result rename operation (openvinotoolkit#4242) * Added result rename operation * Optimize imports * Added ResultRename to package_BOM * ResultRename moved to the end of back phase, code refactoring * Revert incorrect changes * Optimize imports * Added comments and optimized imports. * DPC++ link error workaround. (openvinotoolkit#4192) * DPC++ link error workaround. OpenVINO C++ program failed to link when DPC++ compiler is used. 'make_shared_blob' causes 'unresolved external symbol' error on linking. Commented out some __clang__ specific directives to workaround the issue in "ie_blob.h". * DPC++ compatibility issue fix #2 1. Removed type-by-type template class definition for __clang__. 2. Modified TBlob() destructor. The 'unresolved reference' error occur again if I left 'virtual ~TBlob();' only. It seems it needs to be 'virtual ~TBlob() {};'. * DPC++ compatibility fix #3 - Add DPC++ conditional code Uses '__SYCL_COMPILER_VERSION' predefined macro to check if the compiler is a DPC++ or not. Added conditional directive to switch code based of the detected compiler. NOTE: User program must include <CL/sycl.hpp>, or the '__SYCL_COMPILER_VERSION' macro won't be defined and this fix won't take effect. * DPC++ compatibility issue fix #4 Changed from #ifdef to #if + logical formulas. * DPC++ compatibility issue fix #5 Added compiler check logic in src/ie_rtti.cpp * DPC++ Compatibility issue #6 - ie_parameter.cpp Added compiler check macro for DPC++ to ie_parameter.cpp as well. Co-authored-by: Yasunori Shimura <yasunori.shimura@intel.com> * Azure CI: Disable IB stop * Remove generic ie op (openvinotoolkit#4213) * Removed legacy IE shape infer * Removed GenericIE operation * Removed legacy shape infer tests * Removed legacy test with legacy IE reshape * Fixed compilation issues related to removal of GenericIE * Fixed one more compilation issue with clDNN * Fixed test for reading experimental ops * Updated tests and make IR Reader to load old experimenal and extension ops as opset6 * Change opset of some ops only if they are currently experimental/extension to avoid situation like opset1::Proposal -> opset6::Proposal * Removed more legacy code * Returned back code removed by mistake * Fixed issues related to incorrect merge with master * Merge fixes * Fixed unit tests which starts to fail because now loading the model with unknown operation is failed earlier * Removed incorrectly added code Co-authored-by: Evgeny Lazarev <elazarev.nnov@gmail.com> * [IE CLDNN] Added CTCGreedyDecoderSeqLen operation (openvinotoolkit#4119) * [CPU] Refactors jitters for nGraph interop (openvinotoolkit#4255) * [IE CLDNN] Fixed CTCGreedyDecoderSeqLenLayerTest gpu instances (openvinotoolkit#4326) * Fix comparison of constant with short float NAN values (openvinotoolkit#4299) * fix comparison of constant with short float NAN values * adjust precision, remove elvises * more templates * add ir serialization test with float16 const * remove unused prototxt * [ONNX] Remove linking libonnx from unit-test (openvinotoolkit#4298) * Remove linking libonnx from unit-test * Consider all flavors of protobuf libraries * Avoid passing on NOTFOUND properties * Set system deps for mac * Revert include dirs set up * wider range of unittests added (froze the number) * ONNX RNN/GRU enable dynamic input shape (openvinotoolkit#4241) * Add support for custom ONNX operator PriorBoxClustered (openvinotoolkit#4202) * review comments applied * Force test use bash for setupvars.sh (openvinotoolkit#4321) setupvars.sh is incomatible with sh shell. * enabled skipped unit-test * comment corrections Co-authored-by: Pavel Esir <pavel.esir@intel.com> Co-authored-by: Elizaveta Lobanova <elizaveta.lobanova@intel.com> Co-authored-by: Artyom Anokhov <artyom.anokhov@intel.com> Co-authored-by: Maksim Kutakov <maksim.kutakov@intel.com> Co-authored-by: Maksim Doronin <maksim.doronin@intel.com> Co-authored-by: Alexander Zhogov <alexander.zhogov@intel.com> Co-authored-by: Anastasia Popova <anastasia.popova@intel.com> Co-authored-by: Yasunori Shimura <63567854+yas-sim@users.noreply.github.com> Co-authored-by: Yasunori Shimura <yasunori.shimura@intel.com> Co-authored-by: Evgeny Lazarev <evgeny.lazarev@intel.com> Co-authored-by: Evgeny Lazarev <elazarev.nnov@gmail.com> Co-authored-by: Roman Lyamin <Roman.Lyamin@intel.com> Co-authored-by: Marina Kolpakova <marina.kolpakova@intel.com> Co-authored-by: Vladimir Paramuzov <vladimir.paramuzov@intel.com> Co-authored-by: Bartosz Lesniewski <bartosz.lesniewski@intel.com> Co-authored-by: Tomasz Jankowski <tomasz1.jankowski@intel.com> Co-authored-by: Katarzyna Mitrus <katarzyna.mitrus@intel.com> Co-authored-by: Bartosz Sledz <bartosz.sledz@intel.com> Co-authored-by: Andrey Somsikov <andrey.somsikov@intel.com>

DPC++ link error workaround.

6c4ce3c

OpenVINO C++ program failed to link when DPC++ compiler is used. 'make_shared_blob' causes 'unresolved external symbol' error on linking. Commented out some __clang__ specific directives to workaround the issue in "ie_blob.h".

yas-sim requested a review from a team February 5, 2021 06:11

openvino-pushbot added ExternalIntelPR External contributor from Intel category: inference OpenVINO Runtime library - Inference labels Feb 5, 2021

ilyachur requested a review from ilya-lavrenov February 5, 2021 07:17

ilyachur reviewed Feb 5, 2021

View reviewed changes

DPC++ compatibility issue fix #2

f92dec0

1. Removed type-by-type template class definition for __clang__. 2. Modified TBlob() destructor. The 'unresolved reference' error occur again if I left 'virtual ~TBlob();' only. It seems it needs to be 'virtual ~TBlob() {};'.

dkurt reviewed Feb 8, 2021

View reviewed changes

dkurt reviewed Feb 9, 2021

View reviewed changes

inference-engine/include/ie_blob.h Outdated Show resolved Hide resolved

dkurt reviewed Feb 9, 2021

View reviewed changes

inference-engine/include/ie_blob.h Outdated Show resolved Hide resolved

DPC++ compatibility issue fix #4

f3ca3bd

Changed from #ifdef to #if + logical formulas.

dkurt reviewed Feb 9, 2021

View reviewed changes

inference-engine/include/ie_blob.h Show resolved Hide resolved

DPC++ compatibility issue fix #5

d7a61eb

Added compiler check logic in src/ie_rtti.cpp

dkurt reviewed Feb 9, 2021

View reviewed changes

DPC++ Compatibility issue #6 - ie_parameter.cpp

fc12163

Added compiler check macro for DPC++ to ie_parameter.cpp as well.

ilyachur approved these changes Feb 12, 2021

View reviewed changes

ilya-lavrenov approved these changes Feb 12, 2021

View reviewed changes

ilyachur merged commit 15d6a0f into openvinotoolkit:master Feb 13, 2021

dkurt mentioned this pull request Mar 24, 2021

Remove __clang__ macros from headers #4575

Merged

	#ifdef __clang__
	template <typename T, typename U>
	TBlob<T, U>::~TBlob() {
	free();
	}

Conversation

yas-sim commented Feb 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilya-lavrenov Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yas-sim Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkurt commented Feb 12, 2021

Uh oh!

ilyachur commented Feb 12, 2021

Uh oh!

yas-sim commented Feb 13, 2021

Uh oh!

ilyachur commented Feb 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ilya-lavrenov Feb 9, 2021 •

edited

Loading

yas-sim Feb 10, 2021 •

edited

Loading