[PATCH] D85810: [clang] Pass-through remarks options to lld

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:71
+
+  bool isLLD = llvm::sys::path::filename(LinkerPath) == "ld.lld" ||
+               llvm::sys::path::stem(LinkerPath) != "ld.lld";
----------------
Checking the path is brittle. Consider `Args.getLastArgValue(options::OPT_fuse_ld_EQ) == "lld"`


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:73
+               llvm::sys::path::stem(LinkerPath) != "ld.lld";
+  if (hasMultipleArchs || !isLLD)
+    return false;
----------------
`return isLLD;`

The Mach-O port is a WIP. You don't need to handle `Triple.isOSDarwin() && Args.getAllArgValues(options::OPT_arch).size() > 1;` currently.

This suggests that this simple function should be simply inlined.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:126
+
+  CmdArgs.push_back(Args.MakeArgString(F + Extension));
+
----------------
`(F + Twine(".opt.ld.") + Format).str()`


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:626
+    // handle remark diagnostics on screen options: '-Rpass-*'
+    if (hasRpassOptions(Args)) {
+      renderRpassOptions(Args, CmdArgs);
----------------
http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:631
+    // handle serialized remarks options: '-fsave-optimization-record'
+    // and '-foptimization-record-*'
+    if (willEmitRemarks(Args)) {
----------------
Full stop after a complete sentence.

Capitalize


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:632
+    // and '-foptimization-record-*'
+    if (willEmitRemarks(Args)) {
+      renderRemarksOptions(Args, CmdArgs, ToolChain.getEffectiveTriple(), Input,
----------------
http://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: clang/test/Driver/remarks-pass-through.c:1
+// This test verifies remarks options pass-through into linker(lld)
+
----------------
Consider reusing opt-record.c

The current filename makes it difficult to find related tests of -fsave-optimization-record


================
Comment at: clang/test/Driver/remarks-pass-through.c:24
+// CHECK-CUSTOM: "--opt-remarks-hotness-threshold=100"
+//
+// CHECK-RPASS: "-mllvm" "-pass-remarks=inline"
----------------
Replace `//` with an empty line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810