This is an archive of the discontinued LLVM Phabricator instance.

[lld] add context-sensitive PGO options for MachO
ClosedPublic

Authored by ellis on May 26 2023, 2:03 PM.

Details

Summary

Enable support for CSPGO for lld MachO targets.

Since lld MachO does not support -plugin-opt=, we need to create the --cs-profile-generate and --cs-profile-path= options and propagate them in Darwin.cpp. These flags are not supported by ld64.

Also outline code into getLastCSProfileGenerateArg() to share between CommonArgs.cpp and Darwin.cpp.

CSPGO is already implemented for ELF (https://reviews.llvm.org/D56675) and COFF (https://reviews.llvm.org/D98763).

Diff Detail

Event Timeline

ellis created this revision.May 26 2023, 2:03 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 26 2023, 2:03 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ellis updated this revision to Diff 526199.May 26 2023, 2:32 PM

Only pass flags when using lld

ellis edited the summary of this revision. (Show Details)May 26 2023, 2:32 PM
ellis edited the summary of this revision. (Show Details)
ellis edited the summary of this revision. (Show Details)
ellis published this revision for review.May 26 2023, 2:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2023, 2:34 PM
MaskRay accepted this revision as: MaskRay.May 26 2023, 7:05 PM
MaskRay added inline comments.
clang/lib/Driver/ToolChains/CommonArgs.cpp
1342

Similar code in Clang.cpp addPGOAndCoverageFlags can be replaced by this function.

clang/lib/Driver/ToolChains/Darwin.cpp
459

String literal doesn't need MakeArgString

clang/test/Driver/cspgo-lto.c
8

Prefer --target= to deprecated/legacy (since clang 3.x) -target

lld/MachO/Driver.cpp
1640

This needs a lld/test/MachO test. The ELF patch unfortunately doesn't add a test and I failed to notice it.

This revision is now accepted and ready to land.May 26 2023, 7:05 PM
ellis updated this revision to Diff 526751.May 30 2023, 12:57 PM
ellis marked 3 inline comments as done.

Add test to check that PGOInstrumentationGen(Use) are run

int3 added inline comments.May 30 2023, 2:40 PM
lld/test/MachO/cspgo-gen.ll
13

doesn't *really* matter since you are creating a dylib, but lld-macho doesn't actually treat _start as a special symbol (only main). maybe rename to foo to be less misleading?

ellis updated this revision to Diff 526825.May 30 2023, 3:40 PM
ellis marked 3 inline comments as not done.

Rename test function to foo since _start is not a necessary symbol

ellis updated this revision to Diff 527205.May 31 2023, 2:51 PM

Fix paths in clang test to support windows

This revision was landed with ongoing or failed builds.May 31 2023, 5:53 PM
This revision was automatically updated to reflect the committed changes.