[PATCH] D40917: Emit .debug$H section in clang

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 13 10:48:53 PST 2017


rnk added a subscriber: hans.
rnk added a comment.

I think we'll need some flag for this. A 15% object file size increase is quite a bit, and it will probably slow down linking with MSVC, which is what most Chromium developers are using today. I suspect you'll want to land this, and then @hans will want to roll clang while you're on vacation, and there is some risk that the larger object files will cause problems and he'll need to revert it.. A hidden cl::opt/-mllvm flag would be fine for this, and we can add a real clang flag later (it would add a module flag like /guard:cf).



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:494
 
+  emitTypeGlobalHashes();
+
----------------
I'd check the cl::opt here.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:564-566
+  NamedMDNode *CU_Nodes = MMI->getModule()->getNamedMetadata("llvm.dbg.cu");
+  if (!CU_Nodes)
+    return;
----------------
I bet we can remove this check now that we have the `if (TypeTable.empty()) return;` line. Want to give it a try here and above in `emitTypeInformation`?


================
Comment at: llvm/test/DebugInfo/COFF/global-type-hashes.ll:1
+; RUN: llc -filetype=obj < %s > %t.obj
+; RUN: obj2yaml %t.obj | FileCheck %s --check-prefix=YAML
----------------
I'd do an assembly emission test as well, just to get code coverage of the verbose asm comment code. You only really need to look for .debug$H and a few hashes after that.


https://reviews.llvm.org/D40917





More information about the llvm-commits mailing list