This is an archive of the discontinued LLVM Phabricator instance.

[dwp][libtool-darwin][sancov] Enable llvm-driver
ClosedPublic

Authored by avillega on Jul 31 2023, 4:13 PM.

Details

Summary

Enable llvm-driver for:
llvm-dwp
llvm-libtool-darwin
sancov

Diff Detail

Unit TestsFailed

Event Timeline

avillega created this revision.Jul 31 2023, 4:13 PM
Herald added a project: Restricted Project. · View Herald Transcript
avillega requested review of this revision.Jul 31 2023, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2023, 4:14 PM
avillega edited the summary of this revision. (Show Details)Jul 31 2023, 4:16 PM
phosek accepted this revision.Jul 31 2023, 11:44 PM

LGTM

This revision is now accepted and ready to land.Jul 31 2023, 11:44 PM
avillega updated this revision to Diff 546103.Aug 1 2023, 9:52 AM

Don't enable for dsymutil since it uses target_link_libraries
for APPLE.

avillega retitled this revision from [dsymutil][dwp][libtool-darwin][sancov] Enable llvm-driver to [dwp][libtool-darwin][sancov] Enable llvm-driver.Aug 1 2023, 9:53 AM
avillega edited the summary of this revision. (Show Details)

Don't enable for dsymutil since it uses target_link_libraries
for APPLE.

Ah, because we link against CoreFoundation. I wonder if on Apple platforms, we should just link the driver against it too.

Don't enable for dsymutil since it uses target_link_libraries
for APPLE.

Ah, because we link against CoreFoundation. I wonder if on Apple platforms, we should just link the driver against it too.

Something like that is the plan. Most of the tools left to be added to the driver use target_link_libraries. Need to go through those and conditionally link the driver against those libs when LLVM_TOOL_LLVM_DRIVER_BUILD is enabled.

Something like that is the plan. Most of the tools left to be added to the driver use target_link_libraries. Need to go through those and conditionally link the driver against those libs when LLVM_TOOL_LLVM_DRIVER_BUILD is enabled.

👍

This revision was automatically updated to reflect the committed changes.

I think a better solution would be change the type of linkage from PRIVATE to PUBLIC. To give a concrete example, in https://github.com/llvm/llvm-project/blob/244fd4dfc56a0d59655c65adac6a7258114b8af2/llvm/tools/dsymutil/CMakeLists.txt#L42 you'd use:

target_link_libraries(dsymutil PUBLIC "-framework CoreFoundation")

When driver is not being used, add_llvm_tool bottoms out in add_executable and there's no difference to PRIVATE and PUBLIC, but when driver is enabled add_llvm_tool bottoms out in add_library(... OBJECT ...) and there PUBLIC is correct because it'll make sure that the link flags will be transitively propagated.