[PATCH] D140633: [lld][ARM] don't use short thumb thunks if no branch range extension

Thu Jan 5 08:58:59 PST 2023

stuij marked an inline comment as done.
stuij added a comment.

> Thanks for the manual. It uses "b (2)" which resolves my question. Where does "extra range of 2048 bytes" come from?

The maximum range of a BL is 4MB. So of course the max a ThunkSection can be placed from this BL is also 4MB. At this point we could use a short Thunk which for Thumb on v4/5/6 is a maximum of 2048 bytes  (our friend `b (2)`. But this seems like an unlikely scenario as the ThunkSection would have to be placed exactly 4MB away from the call site, or at least within 2048 bytes from 4MB away to have any effect.

================
Comment at: lld/test/ELF/arm-thumb-range-thunk-os-no-ext.s:4
+// RUN: ld.lld %t -o %t2
+// The output file is large, most of it zeroes. We dissassemble only the
+// parts we need to speed up the test and avoid a large output file
----------------
MaskRay wrote:
> stuij wrote:
> > stuij wrote:
> > > MaskRay wrote:
> > > > The output file has 12+MiB, which is too large. Is it possible to make it smaller with a linker script?
> > > I agree it's not great. I used arm-thumb-range-thunk-os.s as a template which is more than twice as big btw. I'll have a look what I can do.
> > I think an or *the* issue is that the linker will behave differently when using linker script sections, because then the ThunkSection will be placed immediately after the code, which isn't what you want in this case.
> Can you elaborate?
Sure. The `os` in `arm-thumb-range-thunk-os-no-ext.s` stands for `OutputSection`. The goal of the original v7 test is to test range extension thunks in a single section. If you use different sections via a linker script you're not testing how range extension thunks behave in a single section.

Granted, this test setup is definitely overkill for this particular issue, but we're not testing range extension thunks yet for v4/5/6. And it seemed a fitting place to also test that we're doing the correct thing here as well, so I figured why not go the extra mile.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140633