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

Mon Feb 17 23:48:04 PST 2020

MaskRay added a comment.

In D74604#1879493 <https://reviews.llvm.org/D74604#1879493>, @psmith wrote:

> 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

Peter has mentioned most points I wanted to say. If we can think of a good category name, we can place new tests in `arm-reloc-$category.s`.

Use `CHECK-LABEL:` instead of `CHECK:` for `section .R_*` (see ppc32-reloc-addr.s). They can improve FileCheck diagnostics when something changes.

Don't just write `// 0x20000 - . = 56832`. It is unclear what `0x20000` refers to. In `riscv-tls-*`, you can see I write `## rv32: &.got[0] - . = 0x2214 - . = 4096*1+112`

================
Comment at: lld/test/ELF/arm-mov-relocs.s:23
  .section .R_ARM_MOVW_ABS_NC, "ax",%progbits
+ .align 8
  movw r0, :lower16:label
----------------
Is the overalignment (8) significant?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74604