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

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 17 02:36:57 PST 2016


grimar added a comment.

In http://reviews.llvm.org/D17242#353765, @ruiu wrote:

> 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.


That really depends I guess. Should be tested on real app that uses the features of LS. Currently I think it is not in the state when valid tests can be performed.
There are lot of possible ways to optimize I believe, but for now I suggest to leave this one implementation.


================
Comment at: ELF/LinkerScript.cpp:384
@@ -375,3 +383,3 @@
   expect("{");
-  while (!Error && !skip("}"))
+  while (!skip("}"))
     readOutputSectionDescription();
----------------
ruiu wrote:
> Why did you change this?
I changed that when modified readOutputSectionDescription() as well. So this change is to match the style of readOutputSectionDescription() which is called from readSections(). Not sure it is fine for this patch, probably separate patch for both is better. 
That`s why it was changed in separate refactoring patch: http://reviews.llvm.org/D17256.
Point was that there is no need to check Error flag here as skip() does that at first line.

I returned that back as mentioned above refactor patch has it.


http://reviews.llvm.org/D17242





More information about the llvm-commits mailing list