[PATCH] D44780: [ELF] - Implement linker script OVERLAYs.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/ScriptParser.cpp:432
 
+std::vector<BaseCommand *> ScriptParser::readOverlay() {
+  // VA and LMA expressions are optional, though for simplicity of
----------------
This needs a pointer to the page describing the OVERLAY command.


================
Comment at: ELF/ScriptParser.cpp:444-445
+  while (!errorCount() && !consume("}")) {
+    OutputSection *Prev = V.empty() ? nullptr : cast<OutputSection>(V.back());
+    OutputSection *OS = readOverlaySectionDescription(next());
+
----------------
This is more conventional.

  OutputSection *Prev = nullptr;
  for (...) {
    ...
    Prev = OS;
  }



================
Comment at: ELF/ScriptParser.cpp:462
+  Expr MoveDotExpr = [=] {
+    uint64_t MaxSize = 0;
+    for (BaseCommand *Cmd : V)
----------------
Do you have to compute MaxSize inside a lambda?


================
Comment at: ELF/Writer.cpp:2069-2070
+    // addresses, because it is what OVERLAY was designed for.
+    if (Kind == OverlapKind::VirtualAddress && A.Sec->InOverlay &&
+        B.Sec->InOverlay)
+      continue;
----------------
This seems a overdesign. Implementation simplicity is important. Just skip if "InOverlay" is true. Remove OverlapKind enum.


https://reviews.llvm.org/D44780