[PATCH] D20999: [pdbdump] Print out New FPO stream contents.

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: include/llvm/DebugInfo/PDB/Raw/RawTypes.h:83
@@ -82,1 +82,3 @@
 
+// Corresponds to `_FPO_DATA` structure in the PE/COFF spec.
+struct FpoData {
----------------
Do you think this should go in `llvm/Support/COFF.h` (or is it perhaps there already?)

================
Comment at: tools/llvm-pdbdump/LLVMOutputStyle.h:36
@@ -35,2 +35,3 @@
   Error dumpSectionHeaders() override;
+  Error dumpFpoStream() override;
 
----------------
You will probably need to update `YamlOutputStyle.h` as well, you can just have the method do nothing.

================
Comment at: tools/llvm-pdbdump/llvm-pdbdump.cpp:157
@@ -156,1 +156,3 @@
                                  cl::cat(NativeOptions));
+cl::opt<bool> DumpFpo("raw-fpo", cl::desc("dump FPO records"),
+                      cl::cat(NativeOptions));
----------------
I'm starting to wonder if all of this should be broken out into a separate tool.  The whole yaml thing is to support pdb <-> yaml, so maybe it makes sense for `llvm-pdbdump` to continue as the "semantic" pretty dumper, and `llvm-readpdb` as the raw dumper.  This way we could have a more consistent and easy to understand command line option set and not have to worry about option conflicts, and reading/writing pdbs would be the same tool, so be able to share a lot of the same code.

Anyway, I'm just thinking out loud, obvious that is outside the scope of this patch.


http://reviews.llvm.org/D20999