intel / opencl-clang

Other
136 stars 62 forks source link

Add 'clangSerialization' to CMakeLists (fix downstream link error). #475

Closed tylanphear closed 1 year ago

tylanphear commented 1 year ago

The following link error occurs in downstream after 1bda00e358:

ld: error: undefined symbol: clang::PCHContainerOperations::PCHContainerOperations()
>>> referenced by opencl_clang.cpp
>>>               _deps/opencl-clang-build/CMakeFiles/common_clang.dir/opencl_clang.cpp.o:(Compile)
icpx: error: linker command failed with exit code 1 (use -v to see invocation)

This is similar to the link error seen in https://reviews.llvm.org/D157078, which was resolved in the same way (https://reviews.llvm.org/rG36daf3532d91bb3e61d631edceea77ebb8417801).

This patch just adds 'clangSerialization' to the link libraries to resolve the issue.

tylanphear commented 1 year ago

This fix is also urgent to unblock the downstream build. Pinging @hewj03, @haonanya for review.

wenju-he commented 1 year ago

@tylanphear I just take a look at link.txt and it show opencl-clang is linking libclangSerialization.a The root cause is probably that opencl-clang doesn't depends on clangSerialization target and opencl-clang is linking its dependencies before clangSerialization is built. So this build error could happen to other clang static libraries. https://github.com/intel/opencl-clang/commit/1bda00e358ce39e9f3083875b05610b85ec294e3 forgot to use add_dependencies. I suggest to revert https://github.com/intel/opencl-clang/commit/1bda00e358ce39e9f3083875b05610b85ec294e3 first, WDYT?

wwXing0 commented 1 year ago

@tylanphear I just take a look at link.txt and it show opencl-clang is linking libclangSerialization.a The root cause is probably that opencl-clang doesn't depends on clangSerialization target and opencl-clang is linking its dependencies before clangSerialization is built. So this build error could happen to other clang static libraries. 1bda00e forgot to use add_dependencies. I suggest to revert 1bda00e first, WDYT?

reverts commit https://github.com/intel/opencl-clang/commit/1bda00e358ce39e9f3083875b05610b85ec294e3 by https://github.com/intel/opencl-clang/pull/476

tylanphear commented 1 year ago

Closing in favor of #476

wenju-he commented 1 year ago

after discussion with @zhaomaosu, we think this pr is probably the right fix although it is still unclear why clangSerialization is needed twice in link.txt. clangSerialization is also added to some clang tools CMakeLists.txt So I'll revert https://github.com/intel/opencl-clang/pull/476 and then re-open this PR.

wenju-he commented 1 year ago

@tylanphear branch tylanphear:fix_downstream_link_error is deleted, could you please restore the branch and re-open this pr?

tylanphear commented 1 year ago

Reopening as requested by @wenju-he.