[PATCH] D74604: [LLD][ARM] Fix support for SBREL type relocations

Mon Feb 17 11:07:58 PST 2020

psmith added reviewers: grimar, MaskRay.
psmith added a comment.

The code changes look good to me. I'm unfortunately going to have be a pain and ask that you split the additional tests not related to the new relocation support into a separate change, something like "[LLD][ELF][ARM] add missing test cases for ... " that I'd be happy to approve. This helps keep tracking down failures via bisection easier.

I've added a couple more frequent reviewers to see if they have any additional comments.

For the benefit of other reviewers, reference to ELF for the ARM Architecture for definitions of the relocations https://static.docs.arm.com/ihi0044/g/aaelf32.pdf

================
Comment at: lld/test/ELF/arm-mov-relocs.s:32
+// CHECK: 12000:       movw	r0, #0
+// CHECK: 12004:       movw	r1, #4
+// CHECK: 12008:       movw	r2, #12
----------------
Another convention is where the address isn't significant to the line and follows on from the previous instruction is to only put the address on the first instruction. I think it is also the trend to remove the early indentation. For example:
```
// CHECK: 12000: movw  r0, #0
// CHECK:              movw  r1, #4
// CHECK:              movw  r2, #12
// CHECK:              movw  r3, #65532
// CHECK:              movw  r4, #0
```

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74604