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

Sun Apr 12 05:51:17 PDT 2020

thieta marked 2 inline comments as done.
thieta 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
----------------
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.

================
Comment at: lld/test/ELF/arm-thumb-interwork-abs.s:31
+// CHECK: 00021000 <arm_caller>:
+// CHECK-NEXT:    21000: b   #-57352
+// CHECK-NEXT:    21004: bl  #-57356
----------------
psmith wrote:
> thieta wrote:
> > This looked a bit strange to me as I was working through this test. I am not sure if the -57352 is correct here it seems like a very large negative number which is not something I usually see in the other files. If anyone can double check it it would be great.
> The offset looks good to me. The symbol "sym" is defined via defsym to be 0x13001 which is higher than the call site with 4k pages, but lower when using 64k pages. Given that 0x21000 = 135168 in decimal a hand evaluation of the B instruction gives me 135168 + 8 - 57352 = 0x13000 which is what we'd expect the destination address to be for a Thumb defsym of 0x13001.
> 
Thanks for the clarification.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77330