[PATCH] D151589: [lld] add context-sensitive PGO options for MachO

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision as: MaskRay.
MaskRay added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1356
 
+Arg *tools::getLastCSProfileGenerateArg(const ArgList &Args) {
+  auto *CSPGOGenerateArg = Args.getLastArg(options::OPT_fcs_profile_generate,
----------------
Similar code in Clang.cpp `addPGOAndCoverageFlags` can be replaced by this function.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:459
+      llvm::sys::path::append(Path, "default_%m.profraw");
+      CmdArgs.push_back(Args.MakeArgString("--cs-profile-generate"));
+      CmdArgs.push_back(Args.MakeArgString(Twine("--cs-profile-path=") + Path));
----------------
String literal doesn't need `MakeArgString`


================
Comment at: clang/test/Driver/cspgo-lto.c:8
+
+// RUN: %clang -target apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fprofile-use 2>&1 | FileCheck %s --check-prefix=DARWIN-USE1
+// RUN: %clang -target apple-arm64-ios -### %t.o -flto=thin -fuse-ld=lld -fprofile-use=a.profdata 2>&1 | FileCheck %s --check-prefix=DARWIN-USE2
----------------
Prefer `--target=` to deprecated/legacy (since clang 3.x) `-target `


================
Comment at: lld/MachO/Driver.cpp:1640
   config->strictAutoLink = args.hasArg(OPT_strict_auto_link);
+  config->csProfileGenerate = args.hasArg(OPT_cs_profile_generate);
+  config->csProfilePath = args.getLastArgValue(OPT_cs_profile_path);
----------------
This needs a `lld/test/MachO` test. The ELF patch unfortunately doesn't add a test and I failed to notice it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151589/new/

https://reviews.llvm.org/D151589