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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/test/ELF/arm-thumb-range-thunk-os-no-ext.s:2
+// REQUIRES: arm
+// RUN: llvm-mc  -arm-add-build-attributes -filetype=obj -triple=thumbv4t-none-linux-gnueabi %s -o %t
+// RUN: ld.lld %t -o %t2
----------------
remove excess space before `-arm-add-build-attributes`

use `.o` for relocatable files.

some older tests don't use convention, but try fixing them for new tests.


================
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
----------------
stuij wrote:
> 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.
OK, I think the 12MiB file is fine. Change the non-RUN-non-CHECK comments to use `/// `, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140633