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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

This patch looks pretty good.



================
Comment at: COFF/Writer.cpp:298-300
+  // Only ARMNT requires range extension thunks out of the currently supported
+  // architectures.
+  return Config->Machine == ARMNT;
----------------
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.


================
Comment at: COFF/Writer.cpp:333
+
+static bool createThunks(std::vector<Chunk *> &Chunks, int Margin) {
+  bool AddressesChanged = false;
----------------
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.


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


https://reviews.llvm.org/D52156