[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

I have some nits about explicit comments for arguments and default argument values for backwards compatibility. Other than that, it looks like a nice code cleanup.



================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6071
   // Add the target features
-  getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, true);
+  getTargetFeatures(getToolChain(), Triple, Args, CmdArgs, true, false);
 
----------------
This needs `/*ForAS=*/true, /*ForLTOPlugin=*/false)` so it's clear what the booleans are for. Same for other calls to `getTargetFeatures`.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:134
+                       llvm::opt::ArgStringList &CmdArgs, bool ForAS,
+                       bool ForLTOPlugin);
+
----------------
The final argument here should probably default to `false`, so that out-of-tree backends do not break immediately when this patch lands.


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

https://reviews.llvm.org/D67409