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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/llvm-dwp/llvm-dwp.cpp:725-727
+  ArrayRef<std::string> DWOs = DWOFilenames.empty()
+                                   ? ArrayRef<std::string>(InputFiles)
+                                   : ArrayRef<std::string>(DWOFilenames);
----------------
dblaikie wrote:
> 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?
yeah,
i've just looked at binutils dwp ( 2.25.1-22.base.el7).
Actually binutils dwp behaves differently, it takes everything, i.e.

  [alexshap at devvm32123.prn1 ~/fission/build] dwp c.dwo -e out/libFoo.so -o test.dwp
  [alexshap at devvm32123.prn1 ~/fission/build] cat c.cpp
  struct XXXXXX {
    int YYYYYY;
   };

   int ZZZZZZ() {
     XXXXXX x;
     return x.YYYYYY;
   }
   [alexshap at devvm32123.prn1 ~/fission/build] strings test.dwp | grep ZZZZZZ
   ZZZZZZ
   _Z6ZZZZZZv

additionally, binutils dwp allows to pass -e multiple times.

so yes, we can try to replicate this behavior (that looks easy),
alternatively, we can try to avoid introducing this new flag and try to extract the necessary information from the files themselves (i agree, this probably would be cleaner, on the down side - requires more work/a bit more complex logic + switching back and forth from bintutils version to llvm will require changing the invocation)  @dblaikie - how should we proceed ?


Repository:
  rL LLVM

https://reviews.llvm.org/D37371