[PATCH] D90663: [lld-macho] Add very basic support for LTO

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 5 10:17:20 PST 2020


compnerd added inline comments.


================
Comment at: lld/MachO/Driver.cpp:325
+      break;
     newFile = *dylibFile;
     break;
----------------
Why not write this as:

```
if (Optional<DylibFile *> dylib = makeDylibFromTAPI(mbref))
  newFile = *dylib;
```


================
Comment at: lld/MachO/Driver.cpp:334
   }
-  inputFiles.push_back(newFile);
+  if (newFile != nullptr)
+    inputFiles.push_back(newFile);
----------------
LLVM style generally does not compare against `nullptr` or `NULL`.


================
Comment at: lld/MachO/LTO.cpp:40
+  // Provide a resolution to the LTO API for each symbol.
+  for (size_t i = 0, e = objSyms.size(); i != e; ++i) {
+    const lto::InputFile::Symbol &objSym = objSyms[i];
----------------
Could we not just do a `.reserve` on the `std::vector` on L37 and then do an `emplace_back` on the vector and use a range based for loop, and avoid the unnecessary construction and replacement of the resolutions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90663



More information about the llvm-commits mailing list