[PATCH] D17242: [ELF] - Linkerscript KEEP command.

[Rui Ueyama via llvm-commits]

ruiu added a comment.

What's the performance impact of this change? This change made the linker compare all discarded section names against all rules in linker scripts, which can be expensive. Of course that depends on input, but O(n*m) where n is the number of discarded sections and m is the number of section rules is a bit scary.


================
Comment at: ELF/LinkerScript.cpp:46-52
@@ -45,1 +45,9 @@
 
+template <class ELFT>
+bool LinkerScript::shouldKeep(InputSectionBase<ELFT> *S) {
+  for (SectionRule &R : Sections)
+    if (R.match(S))
+      return R.Keep;
+  return false;
+}
+
----------------
You want to define

  template <class ELFT>
  SectionRule *LinkerScript::find(InputSectionBase<ELFT> *S) {
    for (SectionRule &R : Sections)
      if (R.match(S))
        return &R;
    return nullptr;
  }

and use it both from shouldKeep and from getOutputSection.

================
Comment at: ELF/LinkerScript.cpp:384
@@ -375,3 +383,3 @@
   expect("{");
-  while (!Error && !skip("}"))
+  while (!skip("}"))
     readOutputSectionDescription();
----------------
Why did you change this?

================
Comment at: ELF/LinkerScript.cpp:389-396
@@ -380,1 +388,10 @@
 void ScriptParser::readOutputSectionDescription() {
+  auto ReadSections = [this](StringRef OutSec, bool Keep = false) {
+    expect("(");
+    while (!skip(")")) {
+      SectionRule R = { OutSec, next() };
+      R.Keep = Keep;
+      Script->Sections.emplace_back(R);
+    }
+  };
+
----------------
This should be a regular member function rather than a closure.

================
Comment at: ELF/LinkerScript.cpp:413
@@ +412,3 @@
+    } else {
+      error("Unknown command " + Cmd);
+    }
----------------
Please use setError() in the linker script as other code does.


http://reviews.llvm.org/D17242