[PATCH] D26130: [ELF] - Implemented --symbol-ordering-file option.

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

Just some quick observations while building the patch :-)



================
Comment at: ELF/SymbolListFile.cpp:65
+namespace {
+class SymbolOrderingListParser final : public ScriptParserBase {
+public:
----------------
It is not entirely clear you want the ScriptParser infrastructure. For example, do we have to handle quotes?

I would probably make this format dead simple. One symbol per line, symbols with newlines are not supported.



================
Comment at: ELF/Writer.cpp:679
+  // Build sections order map from symbols list.
+  DenseMap<InputSectionBase<ELFT> *, unsigned> SectionsOrder;
+  for (size_t I = 0; I < Config->SymbolOrderingFile.size(); ++I) {
----------------
This is crazy inefficient.

You probably want to make SymbolOrderingFile a DenseMap from CachedStringRef to unsigned.


================
Comment at: test/ELF/symbol-ordering-file.s:9
+# BEFORE:      Contents of section .text:
+# BEFORE-NEXT:  11000 11223344 55
+
----------------
Changing .text to .foo should make this easier to read as .foo doesn't have an implicit alignment.


https://reviews.llvm.org/D26130