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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D43514: ... Rafael Avila de Espindola via Phabricator via cfe-commits
    • [PATCH] D43... Eric Christopher via Phabricator via cfe-commits
    • [PATCH] D43... Rafael Avila de Espindola via Phabricator via cfe-commits
    • [PATCH] D43... Eric Christopher via Phabricator via cfe-commits
      • Re: [PA... Rafael Avila de Espindola via cfe-commits
        • Re:... Eric Christopher via cfe-commits
    • [PATCH] D43... Rafael Avila de Espindola via Phabricator via cfe-commits

Reply via email to