[PATCH] D43514: Start settinng dso_local for COFF

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

Some inline comments - I think this looks ok in general, but I'm curious about the answers to the questions/documentation bits inline.

Thanks!



================
Comment at: lib/CodeGen/CGDecl.cpp:257
 
+  setGVProperties(GV, &D);
+
----------------
If positioning is important here we should probably document it.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:726
+  // Every other GV is local on COFF.
+  // Make an exception for windows OS in the triple: Some firmwares builds use
+  // *-win32-macho triples. This (accidentally?) produced windows relocations
----------------
"firmware"


================
Comment at: lib/CodeGen/CodeGenModule.cpp:733
+
   // Only handle ELF for now.
   if (!TT.isOSBinFormatELF())
----------------
This seems to need an update?


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1650
 
+  CGM.setGVProperties(VTable, RD);
+
----------------
Ditto with the moving comment from above.


https://reviews.llvm.org/D43514