[PATCH] D51894: [codeview] Decode and dump FP regs from S_FRAMEPROC records

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 3 inline comments as done.
rnk added inline comments.


================
Comment at: llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp:63
+  /// Save the machine or CPU type when dumping a compile symbols.
+  Optional<CPUType> MaybeCPUType;
+
----------------
zturner wrote:
> Having the `Optional<>` seems kinda lame.  I think in other places that require "state" in order to dump we just assume that the thing we need is going to be there.  Can we just default it to `x86` if it's not present?
Sure. I feel like I was OCD at first, and then I was lazy for the minimal symbol dumper, but what the heck, less states is better.


================
Comment at: llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp:430-433
+    W.printEnum("LocalFPReg", uint16_t(FrameProc.getLocalFPReg(CPU)),
+                getRegisterNames());
+    W.printEnum("ParamFPReg", uint16_t(FrameProc.getParamFPReg(CPU)),
+                getRegisterNames());
----------------
zturner wrote:
> I keep seeing `FP` and reading "floating point".  Is there any other abbreviation that can remove this ambiguity?
Yeah, that's pretty bad. `getLocalFramePtrReg`?


https://reviews.llvm.org/D51894