[PATCH] D108897: [lld/mac] Set branchRange a bit more carefully

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Arch/ARM64.cpp:137
   thunkSize = sizeof(thunkCode);
-  branchRange = maxIntN(28) - thunkSize;
+  branchRange = 128 * 1024 * 1024;
   stubHelperHeaderSize = sizeof(stubHelperHeaderCode);
----------------
nit: `1 << 28` would be clearer imo


================
Comment at: lld/MachO/ConcatOutputSection.cpp:178
   uint64_t stubsInRangeVA = isecEnd + maxPotentialThunks * target->thunkSize +
                             in.stubs->getSize() - branchRange;
   log("thunks = " + std::to_string(thunkMap.size()) +
----------------
I think the size of the stubs table is factored in here


================
Comment at: lld/MachO/ConcatOutputSection.cpp:258
       uint64_t lowVA = branchRange < callVA ? callVA - branchRange : 0;
-      uint64_t highVA = callVA + branchRange;
+      uint64_t highVA = callVA + branchRange - 4;
       // Calculate our call referent address
----------------
thakis wrote:
> tschuett wrote:
> > thakis wrote:
> > > (See also `AArch64::inBranchRange` in lld/ELF/Arch/AArch64.cpp)
> > Do you have something with more information than a 4. Maybe sizeof() or a named  constant?
> It's really just `1 * 4` in simplified. The range of a two's complement number is -2^n..(2^n - 1), and the call offset is multiplied by 4 to get more range (since function start addresses are required to be 4-aligned on arm64).
the literal constant thing doesn't bother me, but I'm bothered that this is arm64 logic leaking out into generic ConcatOutputSection code. I guess the issue is that the max forward branch range is different from the max backward branch range. How about having `forwardBranchRange` and `backwardBranchRange` instead?


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

https://reviews.llvm.org/D108897