[PATCH] D77330: Consider increasing the default ARM32 page size to 64k.

[Peter Smith via Phabricator via llvm-commits]

psmith added inline comments.


================
Comment at: lld/test/ELF/arm-adr.s:85
+// CHECK:     00020800 <_start>:
 /// 0x11800 + 0x8 - 0x3f8 = 0x11410 = dat1
+// CHECK-NEXT: 20800: sub     r0, pc, #1016
----------------
thieta wrote:
> psmith wrote:
> > Apologies in advance for being a pain. Please could you fix up the comments? PC-relative Arm and Thumb instructions can be a bit strange if you aren't used to them, as the PC-bias of 8 on Arm and 4 on Thumb needs to be accounted for. I wrote a comment hand evaluating the instructions in a lot of these cases to show that the disassembled immediate was correct.
> > 
> > I would hope in the majority of cases, like this one, the fix up is relatively straightforward.
> > 
> > There will be more than just this test affected, at this stage I won't highlight each one. In most of the tests the comments are identified as /// although this is a relatively new convention that may not exist older tests. When updating the comment please could you start with /// rather than //? The reason behind the convention is so that it is easier to pick up comments from FileCheck lines.
> No problem - I can update the tests.
> 
> I am a bit confused about the comment about `/` - did you mean that comments should have three slashes first? or just one? Some tests seems to have comments with `///` but I don't see anything with `/` and this also seems to annoy lit. So I am going to guess you mean `///` - let me know if that's the wrong assumption.
Yes you've got it right; I meant 3 `///` to start a non FileCheck comment.
```
/// 0x11808 + 0x8 + 0x400 = 0x11c10 = dat3
```
There are likely to be older tests that don't follow the new-ish convention, if you have to modify any of them it would be great to update them to follow the new convention.


================
Comment at: lld/test/ELF/arm-branch-rangethunk.s:12
  .globl _start
- .balign 0x10000
+ .balign 0x20000
  .type _start,%function
----------------
thieta wrote:
> @psmith - I just remembered that I didn't get this test to work. @MaskRay told me that the `Inputs/far-long-arm-abs.s` had to be updated as well - but can't say I am sure how exactly.
> 
> So if anyone could show me the exact values or how to calculate them in the input file that would be very helpful.
Looking at the numbers in far-arm-abs.s and far-long-arm-abs.s the values of the symbols are:
far-arm-abs.s
```
too_far1 0x2020008
too_far2 0x202000c
too_far3 0x2020010
```
far-long-arm-abs.s

```
too_far1 0x2020014
too_far2 0x2020018
too_far3 0x202001c
```

The address of the first branch in the old disassembly is 0x20000, for far-arm-abs.s the offset is therefore 0x2020008 - 0x20000 = 0x2000008 which including the 8 byte pc-bias is just out of range of the maximum branch displacement. The short thunks can just about reach these addresses if placed after .text. In far-long-arm-abs.s the addresses are 0x2020014 - 0x20000 = 0x2000014 which is just out of range of the short thunk, but is in range of the long-thunk.

I think that if you alter the addresses of too_far* symbols such that the offset from the first branch instruction matches 0x2000008 for far-arm-abs.s and 0x2000014 for far-long-arm-abs.s then this should work.

While not strictly necessary, there is an opportunity for a small cleanup. The far symbol in far-arm-abs.s is not used by this test, only arm-branch.s. The far symbol in far-long-arm-abs.s is not used at all by any test. It may be worth splitting out far-arm-abs.s into two files?
far-arm-abs.s // just contains the symbol far and is used by arm-branch.s
far-arm-short-abs.s // contains the too_far* symbols from far-arm-abs.s
far-arm-long-abs.s // contains the too_far* symbols from far-long-arm-abs.s with far removed.
 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77330