[PATCH] D67409: [RISCV] enable LTO support, pass some options to linker.
Sam Elliott via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 17 03:56:54 PDT 2019
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
More information about the cfe-commits
mailing list