[PATCH] D24317: Emit S_COMPILE3 CodeView record

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 7 19:42:03 PDT 2016


majnemer added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:574-575
@@ +573,4 @@
+  OS.AddComment("CPUType");
+  CPUType CPU =
+      (getPointerSizeInBytes() == 8) ? CPUType::X64 : CPUType::Pentium3;
+  OS.EmitIntValue(static_cast<uint64_t>(CPU), 2);
----------------
majnemer wrote:
> majnemer wrote:
> > I'd make it a switch on the triple's architecture (`Triple(MMI->getModule()->getTargetTriple()).getArch()`).  Clang also supports ARMNT.
> > 
> > I'd have the default label call `report_fatal_error` to indicate that the architecture is not supported.
> Ah, looks like we can use 0xFFFF for an unknown machine type (https://github.com/Microsoft/microsoft-pdb/blob/e6b1dec61e154b568357537792e1d17a13525d5d/cvdump/dumpsym7.cpp#L256)
Eh, we should just `report_fatal_error`.  Sorry for waffling.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:583-585
@@ +582,5 @@
+    OS.EmitIntValue(V.Part[N], 2);
+  OS.AddComment("Backend version");
+  for (int N = 0; N < 4; ++N)
+    OS.EmitIntValue(V.Part[N], 2);
+
----------------
Apparently, Microsoft tools (like BinScope [https://www.microsoft.com/en-us/download/details.aspx?id=11910]) expect that the backend version is at least 8.0.50727 [1]

I think the backend version should be controlled by LLVM_VERSION_MAJOR, LLVM_VERSION_MINOR and LLVM_VERSION_PATCH instead of whatever is contained in the .ll file.  Because of the aforementioned limitation, perhaps we should do something like `LLVM_VERSION_MAJOR * 1000 + LLVM_VERSION_MINOR * 10 + LLVM_VERSION_PATCH` and use zeros for the other three values.

[1] http://repo.or.cz/nasm.git/commitdiff/1df89ea03902161b76773d0eef48f277b49d918e


https://reviews.llvm.org/D24317





More information about the llvm-commits mailing list