[PATCH] D66279: [ELF] Make LinkerScript::assignAddresses iterative

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Thanks for the update. Have been thinking in the back of my mind if there were any other way of solving the problem without needing to iterate. The only thing I can think of right now is some kind of analysis that, for want of a better word, topologically sorts expressions so that there are no forward references, with an error message if this isn't possible. This could get complicated as there is limited scope to move or reorder assignments in OutputSections. Other than that I've not got any more comments at the moment.



================
Comment at: ELF/LinkerScript.cpp:1065
 
+  DenseMap<const Defined *, std::pair<SectionBase *, uint64_t>> old;
+  for (BaseCommand *base : sectionCommands) {
----------------
It might be worth extracting this and the check below into functions. Something like:
```
oldValues = getSymbolAssignmentValues(sectionCommands);
...
return findChangedSymbol(oldValues);
```
Not a strong opinion though.



================
Comment at: ELF/Writer.cpp:1570
 
   // For some targets, like x86, this loop iterates only once.
+  int assignPasses = 0;
----------------
This comment won't be true anymore, depending on linker scripts.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66279/new/

https://reviews.llvm.org/D66279