[PATCH] D39207: [llvm-objcopy] Add support for dwarf fission

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 2 03:50:48 PDT 2017


jhenderson added a comment.

Couple more small things, and then I think we're done.



================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:72-73
+    SplitDWO("split-dwo",
+             cl::desc("equivalent to strip-dwo on the input file, then "
+                      "extract-dwo on the input file to <dwo-file>"),
+             cl::value_desc("dwo-file"));
----------------
Reading again, I think these are the wrong way around - the extract needs to happen first, otherwise there's nothing to extract!

Text should read "equivalent to extract-dwo on the input file to <dwo-file>, then strip-dwo on the input file".


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:148-149
+
+  if (!SplitDWO.empty())
+    SplitDWOToFile<ELF64LE>(ObjFile, SplitDWO.getValue());
+
----------------
To be absolutely clear, I'd move this clause to before the RemovePred assignments, either to about line 128 (in the current diff), or to line 137, depending on your preference. The reason for this is to maintain the conceptual order of things - split the DWO into a separate file needs to happen before the stripping. Really, it's an independent operation, so should not be mixed in with all the remove section stuff.


Repository:
  rL LLVM

https://reviews.llvm.org/D39207





More information about the llvm-commits mailing list