[PATCH] D38776: [codeview] Implement FPO data assembler directives

[David Majnemer via Phabricator via llvm-commits]

majnemer accepted this revision.
majnemer added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp:264
+    switch (LLVMReg) {
+    // MSVC only seems to emit symbolic register names for EIP, EBP, and ESP,
+    // but the format seems to support more than that, so we emit them.
----------------
rnk wrote:
> majnemer wrote:
> > Comment needs to be updated now that you added more symbolic names.
> I did update it, it now documents that we differ from MSVC here.
Misread it, sorry :(


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp:307-309
+  // Each saved register is stored at an unchanging negative CFA offset.
+  for (RegSaveOffset RO : RegSaveOffsets)
+    FuncOS << printFPOReg(MRI, RO.Reg) << " $T0 " << RO.Offset << " - ^ = ";
----------------
Are these sorted by register? If not, you could get more string sharing if you sorted first.


https://reviews.llvm.org/D38776