[PATCH] D52156: [LLD] [COFF] Alternative ARM range thunk algorithm

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: COFF/Writer.cpp:298-300
+  // Only ARMNT requires range extension thunks out of the currently supported
+  // architectures.
+  return Config->Machine == ARMNT;
----------------
ruiu wrote:
> I would inline this function because it is too small, and I don't see a need to abstract this predicate this way at the moment.
Ok - at some point I think I had more uses of it and the inline function made the check clearer, but yes, with the one current use there's little point in it - I'll inline it.


================
Comment at: COFF/Writer.cpp:333
+
+static bool createThunks(std::vector<Chunk *> &Chunks, int Margin) {
+  bool AddressesChanged = false;
----------------
ruiu wrote:
> Can you explain the algorithm of thunk creation, that is, we create thunks if a jump target looks too far with some margin, and then after creating all thunks, verify that all jump targets are within their ranges, and if not, redo with a wider margin.
Ok, will do.


================
Comment at: COFF/Writer.cpp:425
+
+  int Pass = 0, Margin = 1024 * 100;
+  while (true) {
----------------
ruiu wrote:
> nit: we usually define only one variable in one line.
Will fix


https://reviews.llvm.org/D52156