[PATCH] D24317: Emit S_COMPILE3 CodeView record

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 9 16:21:05 PDT 2016


majnemer added inline comments.

================
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);
+
----------------
amccarth wrote:
> 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.
Eh, but the frontend is the IR that came from clang and the backend is AsmPrinter's job.
The test issue can be handled by making the FileCheck use a regex.


https://reviews.llvm.org/D24317





More information about the llvm-commits mailing list