[PATCH] D22749: [ELF] - Linkerscript: Implemented SORT command

[Ed Maste via llvm-commits]

emaste added a comment.

> I don't know yet, but we need to do something. It's probably nice to slow down a bit and make the code simpler and easier to read.


One little comment, we're now quite close to fully handling the FreeBSD kernel linker script.

What's missing:

1. AT (https://reviews.llvm.org/D19272)
2. SORT (https://reviews.llvm.org/D22749)
3. Filename specification (e.g. `KEEP (*crtbegin.o(.ctors))`)
4. EXCLUDE_FILE (https://reviews.llvm.org/D22795)
5. CONSTRUCTORS
6. `. =` assignment in output section (PR28720)

I think `CONSTRUCTORS` is just a nop for ELF (and we can delete it from the linker script) and PR28720 can also be addressed easily in FreeBSD's linker script instead of changing lld.

At least as far as the FreeBSD base system and kernel is concerned there's not much left to implement, regardless of whether we pause now to refactor and clean up or not.


================
Comment at: ELF/LinkerScript.cpp:139
@@ +138,3 @@
+            });
+      for (InputSectionBase<ELFT> *I : Candidates)
+        AddInputSec(I, OutCmd->Name, OutCmd->Constraint);
----------------
grimar wrote:
> emaste wrote:
> > My comment crossed your update. To be clear, I think the current version of this block is correct.
> Yep, I understood. Thanks !
> Btw I think I never saw sorting by filenames in scripts. I think it is not common and we can postpone the support. What do you think ?
Yes I think it's sensible to postpone support for sorting by filename.


https://reviews.llvm.org/D22749