[PATCH] D70577: [Remarks][LTO] Infer remarks file path from -object_path_lto

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg created this revision.
thegameg added reviewers: arphaman, dexonsmith, steven_wu.
Herald added subscribers: inglorion, mehdi_amini.
Herald added a reviewer: JDevlieghere.

In the common flow, remarks are expected to be in the .dSYM bundle after dsymutil collects them from the object files and creates a standalone remark file. After that, the linked binary and the .dSYM bundle are sufficient to execute, debug, and process the remarks.

Even if users pass -object_path_lto to the linker, we generate the temporary remark files based on the output file requested through `-o`, which is very inconvenient when users try to separate their intermediate products from their final products. This will leave intermediate remarks in the final products, like Frameworks or other bundles.

This patch will change that behavior to try to infer the remarks file path from the path specified in -object_path_lto.

It is a little messy as the driver will try to read flags that are intended to be passed to the linker, but for now, it's the only option and `hasExportSymbolDirective` is already doing this.

The cleaner way to do this would be to add an interface for the linker to specify the remark output file when setting up LTO.


https://reviews.llvm.org/D70577

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld.c


Index: clang/test/Driver/darwin-ld.c
===================================================================
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -315,6 +315,20 @@
 // PASS_REMARKS_OUTPUT: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml"
 // PASS_REMARKS_OUTPUT-NOT: -lto-pass-remarks-with-hotness
 
+// Check that when -object_path_lto is used, we're passing the right path to -lto-pass-remarks-output.
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### -o foo/bar.out -Wl,-object_path_lto,foo/bar_lto.o 2> %t.log
+// RUN: FileCheck -check-prefix=REMARKS_OBJECT_PATH_LTO_WL %s < %t.log
+// REMARKS_OBJECT_PATH_LTO_WL: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar_lto.o.opt.yaml"
+// Also check for -Xlinker.
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### -o foo/bar.out -Xlinker -object_path_lto -Xlinker foo/bar_lto.o 2> %t.log
+// RUN: FileCheck -check-prefix=REMARKS_OBJECT_PATH_LTO_XLINKER %s < %t.log
+// REMARKS_OBJECT_PATH_LTO_XLINKER: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar_lto.o.opt.yaml"
+// Make sure that we pick the path based on -o if there is no second -Xlinker
+// with the path after -object_path_lto.
+// RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### -o foo/bar.out -Xlinker -object_path_lto 2> %t.log
+// RUN: FileCheck -check-prefix=REMARKS_OBJECT_PATH_LTO_XLINKER_MISSING %s < %t.log
+// REMARKS_OBJECT_PATH_LTO_XLINKER_MISSING: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "foo/bar.out.opt.yaml"
+
 // RUN: %clang -target x86_64-apple-darwin12 %t.o -fsave-optimization-record -### 2> %t.log
 // RUN: FileCheck -check-prefix=PASS_REMARKS_OUTPUT_NO_O %s < %t.log
 // PASS_REMARKS_OUTPUT_NO_O: "-mllvm" "-lto-pass-remarks-output" "-mllvm" "a.out.opt.yaml"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -427,6 +427,40 @@
   return Args.hasArg(options::OPT_fobjc_link_runtime);
 }
 
+/// Check if the link command contains a path for the lto'd object file.
+static Optional<StringRef> getObjectPathLTO(const ArgList &Args) {
+  bool ExpectPathNext = false;
+  for (Arg *A : Args) {
+    if (ExpectPathNext) {
+      // Expecting -Xlinker <path>, but it's not there.
+      if (!A->getOption().matches(options::OPT_Xlinker))
+        return None;
+      return StringRef(A->getValue(0));
+    }
+
+    bool isWl = A->getOption().matches(options::OPT_Wl_COMMA);
+    bool isXlinker = A->getOption().matches(options::OPT_Xlinker);
+    // We're looking for either:
+    // * -Wl,-object_path_lto,<path>
+    // or
+    // * -Xlinker -object_path_lto -Xlinker <path>
+    if (!isWl && !isXlinker)
+      continue;
+
+    if (!A->containsValue("-object_path_lto"))
+      continue;
+
+    // In case of -Wl, the path is in the same argument.
+    if (isWl)
+      return StringRef(A->getValue(1));
+
+    // In case of -Xlinker, we need to catch the next -Xlinker option and get
+    // the path from there.
+    ExpectPathNext = true;
+  }
+  return None;
+}
+
 void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                   const InputInfo &Output,
                                   const InputInfoList &Inputs,
@@ -471,7 +505,11 @@
     CmdArgs.push_back("-mllvm");
 
     SmallString<128> F;
-    F = Output.getFilename();
+    if (Optional<StringRef> Filename = getObjectPathLTO(Args))
+      F = *Filename;
+    else
+      F = Output.getFilename();
+
     F += ".opt.";
     if (const Arg *A =
             Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -360,6 +360,11 @@
    serialization format is specified, the file will be named
    "foo.opt.<format>".
 
+   On Darwin platforms, if ``-flto`` is used and the file path is not provided,
+   the output file will be chosen according to the linker argument
+   ``-object_path_lto`` if present, or according to the output file specified
+   through ``-o`` otherwise.
+
 .. _opt_foptimization-record-passes:
 
 **-foptimization-record-passes**


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70577.230566.patch
Type: text/x-patch
Size: 4369 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191122/ca33feee/attachment.bin>