This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Fix building the second import library when built in the main llvm build root
AbandonedPublic

Authored by mstorsjo on Mar 26 2023, 2:58 PM.

Details

Summary

Pass an absolute path to initial import library in this case, to
make it clear which file is intended. (For the generated build
files, cmake still uses relative paths as suitable.)

The issue was that the initial import library was stored in
ARCHIVE_OUTPUT_DIRECTORY as lib/libomp.dll.lib, while the same
file added as a source file to the second import library was
interpreted as runtime/src/libomp.dll.lib.

This is an alternative to D146894.

Diff Detail

Event Timeline

mstorsjo created this revision.Mar 26 2023, 2:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2023, 2:58 PM
mstorsjo requested review of this revision.Mar 26 2023, 2:58 PM
Herald added a project: Restricted Project. · View Herald Transcript

I will be good with this solution, but I think there could be a simpler one in the end. While CMAKE does wants to see a source, that does not have to be a cpp (which is what's causing this need to unnecessarily feed the first step's library to satisfy lib's attempt at def symbols resolution). There are other source types (e.g., a. txt) which will satisfy the source presence requirement but which CMAKE will filter out from the librarian generating the right command line for it.

I will submit a patch with this for review to https://reviews.llvm.org/D146894.

I will be good with this solution, but I think there could be a simpler one in the end. While CMAKE does wants to see a source, that does not have to be a cpp (which is what's causing this need to unnecessarily feed the first step's library to satisfy lib's attempt at def symbols resolution). There are other source types (e.g., a. txt) which will satisfy the source presence requirement but which CMAKE will filter out from the librarian generating the right command line for it.

Oh, nice, if that works, that's probably even better!

I did try to convert the import library generation to a cmake custom target, and I got it working, but I'm not entirely convinced that it's any better than what we have right now.

mstorsjo abandoned this revision.Mar 28 2023, 12:41 AM

Superseded by the improved version of D146894.