[PATCH] D119049: [LLD] Allow usage of LLD as a library
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 9 15:42:01 PST 2023
MaskRay added a comment.
Thanks for the patch! I have gone through the code and it looks in a good shape. Left some suggestions.
================
Comment at: lld/Common/DriversMain.cpp:60
+ // to allow detecting the -m argument from arguments in them.
+ SmallVector<const char *, 256> expandedArgs(args.data(),
+ args.data() + args.size());
----------------
Hopefully share with other template instantiations to decrease the executable size.
================
Comment at: lld/Common/DriversMain.cpp:152
+namespace lld {
+bool inTestOutputDisabled = false;
+}
----------------
================
Comment at: lld/Common/DriversMain.cpp:178
+namespace lld {
+Result lldMain(llvm::ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
+ llvm::raw_ostream &stderrOS, llvm::ArrayRef<DriverDef> drivers,
----------------
`lld::lldMain` (see https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions)
Fix other places as well.
================
Comment at: lld/ELF/Driver.cpp:111
+namespace elf {
+bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
+ llvm::raw_ostream &stderrOS, bool exitEarly, bool disableOutput) {
----------------
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
================
Comment at: lld/ELF/Options.td:739
+// Flag that selects the driver, it should be ignored.
+def flavor : Separate<["-"], "flavor">;
----------------
We should avoid ignoring `-flavor` in all drivers. It's probably not difficult to strip `-flavor` from the driver dispatcher.
================
Comment at: lld/MachO/Driver.cpp:1367
+namespace macho {
+bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
+ llvm::raw_ostream &stderrOS, bool exitEarly, bool disableOutput) {
----------------
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
================
Comment at: lld/unittests/AsLibAll/AllDrivers.cpp:18
+
+bool lldInvoke(std::vector<const char *> args) {
+ args.push_back("--version");
----------------
`static`
================
Comment at: lld/unittests/AsLibAll/CMakeLists.txt:1
+add_lld_unittests(LLDAsLibAllTests
+ AllDrivers.cpp
----------------
Add a file-level comment explaining what `AsLibAll` means.
================
Comment at: lld/unittests/AsLibELF/CMakeLists.txt:1
+add_lld_unittests(LLDAsLibELFTests
+ ROCm.cpp
----------------
Add a file-level comment explaining what `AsLibELF` means.
================
Comment at: lld/unittests/AsLibELF/ROCm.cpp:8
+//===----------------------------------------------------------------------===//
+
+#include "lld/Common/Driver.h"
----------------
Add a comment about the purpose.
================
Comment at: lld/unittests/AsLibELF/ROCm.cpp:21
+ if (llvm::StringRef(path).contains("%")) {
+ expanded.replace(expanded.find("%S"), 2, thisFile.data(), thisFile.size());
+ }
----------------
remove braces
================
Comment at: lld/unittests/AsLibELF/ROCm.cpp:28
+
+bool lldInvoke(const char *inPath, const char *outPath) {
+ std::vector<const char *> args{"ld.lld", "-shared", inPath, "-o", outPath};
----------------
`static`
================
Comment at: lld/unittests/AsLibELF/SomeDrivers.cpp:12
+
+LLD_HAS_DRIVER(elf);
+
----------------
`static`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119049/new/
https://reviews.llvm.org/D119049
More information about the llvm-commits
mailing list