[PATCH] D24317: Emit S_COMPILE3 CodeView record

[Adrian McCarthy via llvm-commits]

amccarth marked 2 inline comments as done.
amccarth added a comment.

It looks like I have to fix up more tests, too, because the extra record make the subsection size a little larger.  I'm working through those now but may not have a new diff until Monday.


================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:530-532
@@ +529,5 @@
+
+struct Version {
+  int Part[4];
+};
+
----------------
majnemer wrote:
> This should be in the anonymous namespace.
oops.  I had it in there initially until I learned that LLVM style is to prefer file static functions to functions in anonymous namespaces.

================
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);
+
----------------
majnemer wrote:
> 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
I see your point, but ...

1.  I also have to provide the textual compiler version string, and it seems awkward to get that from a different source than the structured data.  This seems prone to getting conflicting information.

2.  Having the version dependent on the actual build instead of what's in the .ll file makes the test harder to write.

What if I continue to parse the version from the string but roll up the fields as you've described to solve the Binscope expectation problem?

Another possibility is to use the MS compatibility version rather than the Clang version and synthesize a string like "clang in MSVC compatibility mode 1.2.3.4" for the textual compiler version.


https://reviews.llvm.org/D24317