[PATCH] [LLD] Add a new PE/COFF port

[Davide Italiano]

The overall patch looks very good to me. Some comments, don't take them as mandatory just suggestions.
Thanks!


================
Comment at: COFF/Chunks.cpp:46
@@ +45,3 @@
+bool SectionChunk::isRoot() {
+  if (isCOMDAT())
+    return false;
----------------
Can you add a small comment here explaining why COMDATs are GC roots?

================
Comment at: COFF/Chunks.cpp:72
@@ +71,3 @@
+  // Mark associative sections if any.
+  for (Chunk *C : AssocChildren)
+    C->markLive();
----------------
using 'auto' here? Or do you think it makes the code less readable?

================
Comment at: COFF/Chunks.cpp:131
@@ +130,3 @@
+  uint32_t E = File->getCOFFObj()->getNumberOfSymbols();
+  for (uint32_t I = 0; I < E; ++I) {
+    auto SrefOrErr = File->getCOFFObj()->getSymbol(I);
----------------
In other places you assign E in the for() initialization, any reason you don't do that here as well? Maybe there are few other places where you do this that I missed. 

================
Comment at: COFF/InputFiles.cpp:111
@@ +110,3 @@
+  } else {
+    return make_dynamic_error_code(Twine(Name) + " is not a COFF file.");
+  }
----------------
No braces needed, I guess.

================
Comment at: COFF/Memory.h:29
@@ +28,3 @@
+    P[S.size()] = '\0';
+    return StringRef(P, S.size());
+  }
----------------
I think it may make sense to make this function (or set of function(s)) available everywhere, rather than making them specific to lld.
I can imagine the usefulness of having a facility to return a NULL-terminated copy of a string somewhere else in the tree.

================
Comment at: COFF/Options.td:21
@@ +20,3 @@
+def defaultlib : P<"defaultlib", "Add the library to the list of input files">;
+def nodefaultlib : P<"nodefaultlib", "Remove a default library">;
+def disallowlib : Joined<["/", "-", "-?"], "disallowlib:">, Alias<nodefaultlib>;
----------------
Can you sort these (alphabetically?)

================
Comment at: COFF/Writer.cpp:30
@@ +29,3 @@
+static const int PageSize = 4096;
+static const int FileAlignment = 512;
+static const int SectionAlignment = 4096;
----------------
I personally prefer to express power-of-two constant(s) as 1 << SOMETHING; but it's not a strong preference.

http://reviews.llvm.org/D10036

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/