[PATCH] D97550: [LLD][ELF][ARM] Refactor inBranchRange to use addend for PC Bias

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for working on this! `inBranchRange` looks simpler now.



================
Comment at: lld/ELF/Arch/ARM.cpp:359
+  else
+    // Bit 0 == 1 denotes Thumb state, it is not part of the range
+    dst &= ~0x1;
----------------
Nit: add a period after a complete sentence.


================
Comment at: lld/ELF/Arch/ARM.cpp:391
-
-  uint64_t distance = (src > dst) ? src - dst : dst - src;
-  return distance <= range;
----------------
Thanks for the clean-up!

I stared at this piece code a couple of times and worried that this could be off by a few bytes but was not able to comprehend it.


================
Comment at: lld/ELF/Relocations.cpp:1937
+  // Compensate for the different ARM/Thumb PC bias so we can reuse more thunks.
+  int64_t keyAddend = rel.addend + getPCBias(rel.type);
 
----------------
This probably needs a test (similar to D70637) to test sharing, even if it is difficult to construct a test exercising the range limit.


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

https://reviews.llvm.org/D97550