[PATCH] D82812: [llvm-install-name-tool] Merge rpath with id/change

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 1 02:39:51 PDT 2020


alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:164-167
+  for (LoadCommand &LC : Obj.LoadCommands) {
+    if (LC.MachOLoadCommand.load_command_data.cmd == MachO::LC_RPATH)
+      OriginalRPaths.insert(getPayloadString(LC));
+  }
----------------
jhenderson wrote:
> I thought you were avoiding iterating twice over the LoadCommands before? Why are this loop and the update loop below separate?
@jhenderson - I think this was my suggestion, imo splitting these steps makes the code easier to reason about.
>From the performance perspective it doesn't matter much, even huge binaries have ~70 load commands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82812





More information about the llvm-commits mailing list