[PATCH] D37371: [llvm-dwp] Add command line option "-e"

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: test/tools/llvm-dwp/X86/dwos_list_from_exec_simple.test:4
+
+FIXME: For some reason, piping straight from llvm-dwp to llvm-dwarfdump doesn't behave well - looks like dwarfdump is reading/closes before dwp has finished.
+
----------------
Yeah, it's difficult/not quite possible to stream out ELF files (because the header has to point to the section headers, which are emitted at the end of the file so they can contain all the section sizes... (you could do a prepass to compute all these things then you could stream output)) - Clang (& maybe the llvm tools like llvm-mc, etc - not sure, haven't checked them) avoids this by writing to a temp file then streaming the temp file out through stdout or whatnot. llvm-dwp doesn't have that functionality, but could be taught it, maybe. (might be useful if it were optional, for those who don't want to burn more file IO/space, etc)


================
Comment at: tools/llvm-dwp/llvm-dwp.cpp:57-60
+static opt<std::string> ExecFilename(
+    cl::Optional, "e",
+    desc("Specify the executable/library file to get the list of *.dwo from"),
+    value_desc("filename"), cat(DwpCategory));
----------------
Wonder if this even needs a separate argument - would it be reasonable to detect which case this is in? (probably not by file extension, but by contents? Not sure... maybe that'd be hard/annoying/not much use)


================
Comment at: tools/llvm-dwp/llvm-dwp.cpp:725-727
+  ArrayRef<std::string> DWOs = DWOFilenames.empty()
+                                   ? ArrayRef<std::string>(InputFiles)
+                                   : ArrayRef<std::string>(DWOFilenames);
----------------
Seems weird to ignore all the other inputs silently if -e is specified. Maybe error out if there are both?

Alternatively could support both, maybe - more like what I mentioned above, even. Could detect that something is an executable/contains skeleton units - use those to find DWOs, and treat that as another source of DWOs (so you could have arbitrarily many executables and DWOs on the command line and they'd all be read and smooshed into a DWP).

*shrug* Dunno.

What does binutils dwp do if you have both -e and .dwo files? Error?


Repository:
  rL LLVM

https://reviews.llvm.org/D37371