[ARM][MC][Thumb] Revert relocation for some pc-relative fixups.

[MarkMurrayARM]

Summary

This commit reverts https://reviews.llvm.org/D75039. Concensus appears to
be in favour of assembly-time resolution of these ADR and LDR relocations,
in line with GNU.

[peter.smith]

That is my understanding too from my comment in https://reviews.llvm.org/D72892#1923502
I have removed the tests from LLD that used the relocations coming from these instructions and have replaced them with .inst and .reloc. Will be worth making sure that ninja check-lld passes with this change applies. I'm happy for this to be applied. Will need to check with the other reviewers to make sure they are happy.

[MaskRay]

Interestingly I received some feedback from my earlier GNU as bug report yesterday (https://sourceware.org/bugzilla/show_bug.cgi?id=25406). GNU as will not emit relocations.
I still want to check whether they think rejecting non-STB_LOCAL labels in a potentially preemptible context is acceptable.
I remember that we've seen such reports twice. One is some FreeBSD ARM folks (but at least another two FreeBSD folks told me I can persist). One is a Linux kernel assembly which may have been fixed now. I should add that allowing some Linux kernel in LLVM 9/10 while rejecting some in LLVM 10/11 is ok. There are still a few assembly files not assembled with MC. So being more rigid than GNU as may not cause churn/perceivable problems to Linux kernel builders.

For the patch itself, I am now ok with either way. But please make use of this opportunity to clean up/enhance tests, rather than perform a simple revert.

[MaskRay]

[ARM][MD] Revert relocation for some pc-relative fixups.

Did you mean MC? Also please mention Thumb. It greatly improves the information conveyed by the commit subject with just 6 more characters.

[MarkMurrayARM]

Improve commit message slightly.

[MaskRay]

LGTM, but I'll defer the approval to Peter.

llvm/test/MC/ARM/thumb1-relax-adr.s

8

delete empty line

llvm/test/MC/ARM/thumb1-relax-ldrlit.s

8

delete empty line

[psmith]

LGTM with MaskRay's test comments addressed.

[MarkMurrayARM]

Address review nits.

[thakis]

This broke check-lld everywhere, e.g. http://lab.llvm.org:8011/builders/lld-x86_64-win/builds/71/steps/test-check-all/logs/FAIL%3A%20lld%3A%3Aarm-thumb-undefined-weak.s or http://45.33.8.238/linux/15676/step_11.txt

Please take a look, and revert while you investigate if it's not an easy fix.

[psmith]

In D78301#1992038, @thakis wrote:

This broke check-lld everywhere, e.g. http://lab.llvm.org:8011/builders/lld-x86_64-win/builds/71/steps/test-check-all/logs/FAIL%3A%20lld%3A%3Aarm-thumb-undefined-weak.s or http://45.33.8.238/linux/15676/step_11.txt

Please take a look, and revert while you investigate if it's not an easy fix.

If you can give me a an hour or two I can fix, am away from computer at moment. The LLD test will need the adr and ldr instructions replaced with .inst and .reloc I thought I'd done all of these in arm-thumb-adr.s and arm-thumb-ldrlit.s but it looks like I missed this one.

[psmith]

In D78301#1992076, @psmith wrote:

In D78301#1992038, @thakis wrote:

This broke check-lld everywhere, e.g. http://lab.llvm.org:8011/builders/lld-x86_64-win/builds/71/steps/test-check-all/logs/FAIL%3A%20lld%3A%3Aarm-thumb-undefined-weak.s or http://45.33.8.238/linux/15676/step_11.txt

Please take a look, and revert while you investigate if it's not an easy fix.

If you can give me a an hour or two I can fix, am away from computer at moment. The LLD test will need the adr and ldr instructions replaced with .inst and .reloc I thought I'd done all of these in arm-thumb-adr.s and arm-thumb-ldrlit.s but it looks like I missed this one.

Committed fix to LLD tests under 61bccda9d9d920c72f49025f11e8601daeb096ec and checked it worked with this patch.

[MaskRay]

@MarkMurrayARM For MC changes, you need ninja check-llvm check-lld. From my experience most affected tests are under llvm/test/{CodeGen,MC,DebugInfo} lld/test.

llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp

unsigned IsPCRelConstant =
       MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_Constant;
   const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
       //
       // Name                      Offset (bits) Size (bits)     Flags
       {"fixup_arm_ldst_pcrel_12", 0, 32, IsPCRelConstant},
       {"fixup_t2_ldst_pcrel_12", 0, 32,
-       MCFixupKindInfo::FKF_IsPCRel |
-           MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+       IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_pcrel_10_unscaled", 0, 32, IsPCRelConstant},
       {"fixup_arm_pcrel_10", 0, 32, IsPCRelConstant},
       {"fixup_t2_pcrel_10", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |
            MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_pcrel_9", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_pcrel_9", 0, 32,
        IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_thumb_adr_pcrel_10", 0, 8,
-       MCFixupKindInfo::FKF_IsPCRel |
-           MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+       IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_adr_pcrel_12", 0, 32, IsPCRelConstant},
       {"fixup_t2_adr_pcrel_12", 0, 32,
-       MCFixupKindInfo::FKF_IsPCRel |
-           MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
+       IsPCRelConstant | MCFixupKindInfo::FKF_IsAlignedDownTo32Bits},
       {"fixup_arm_condbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_uncondbranch", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_condbranch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_uncondbranch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_thumb_br", 0, 16, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_uncondbl", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_condbl", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_arm_blx", 0, 24, MCFixupKindInfo::FKF_IsPCRel},
const static MCFixupKindInfo InfosLE[ARM::NumTargetFixupKinds] = {
       {"fixup_arm_mod_imm", 0, 12, 0},
       {"fixup_t2_so_imm", 0, 26, 0},
       {"fixup_bf_branch", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bf_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfl_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfc_target", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_bfcsel_else_target", 0, 32, 0},
       {"fixup_wls", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
-      {"fixup_le", 0, 32, MCFixupKindInfo::FKF_IsPCRel}};
+      {"fixup_le", 0, 32, MCFixupKindInfo::FKF_IsPCRel}
+  };
   const static MCFixupKindInfo InfosBE[ARM::NumTargetFixupKinds] = {
       // This table *must* be in the order that the fixup_* kinds are defined in
       // ARMFixupKinds.h.
       //
       // Name                      Offset (bits) Size (bits)     Flags
       {"fixup_arm_ldst_pcrel_12", 0, 32, MCFixupKindInfo::FKF_IsPCRel},
       {"fixup_t2_ldst_pcrel_12", 0, 32,
        MCFixupKindInfo::FKF_IsPCRel |

llvm/lib/Target/ARM/MCTargetDesc/ARMELFObjectWriter.cpp

if (IsPCRel) {
     case ARM::fixup_arm_thumb_bl:
     case ARM::fixup_arm_thumb_blx:
       switch (Modifier) {
       case MCSymbolRefExpr::VK_TLSCALL:
         return ELF::R_ARM_THM_TLS_CALL;
       default:
         return ELF::R_ARM_THM_CALL;
       }
-    case ARM::fixup_thumb_adr_pcrel_10:
-    case ARM::fixup_arm_thumb_cp:
-      return ELF::R_ARM_THM_PC8;
-    case ARM::fixup_t2_adr_pcrel_12:
-      return ELF::R_ARM_THM_ALU_PREL_11_0;
-    case ARM::fixup_t2_ldst_pcrel_12:
-      return ELF::R_ARM_THM_PC12;
     case ARM::fixup_bf_target:
       return ELF::R_ARM_THM_BF16;
     case ARM::fixup_bfc_target:
       return ELF::R_ARM_THM_BF12;
     case ARM::fixup_bfl_target:
       return ELF::R_ARM_THM_BF18;
     }
   }

llvm/test/MC/ARM/pcrel-global-rel.s

-@ RUN: llvm-mc -filetype=obj -triple=armv7 %s -o %t
-@ RUN: llvm-readobj -r %t
-
-@ Check that for ELF targets we generate a relocation for a within section
-@ pc-relative reference to a global symbol as it may be interposed and we won't
-@ know till link time whether this is possible.
-.thumb
-.thumb_func
-
-.globl bar
-bar:
-adr r0, bar      @ thumb_adr_pcrel_10
-adr.w r0, bar    @ t2_adr_pcrel_12
-ldr.w pc, bar    @ t2_ldst_pcrel_12
-
-@ CHECK:      0x0 R_ARM_THM_ALU_PREL_11_0 bar 0x0
-@ CHECK-NEXT: 0x4 R_ARM_THM_ALU_PREL_11_0 bar 0x0
-@ CHECK-NEXT: 0x8 R_ARM_THM_PC12 bar 0x0

llvm/test/MC/ARM/pcrel-global.s

 @ RUN: llvm-mc -filetype=obj -triple=armv7 %s -o %t
 @ RUN: llvm-readelf -r %t | FileCheck %s
 
 @ CHECK: There are no relocations in this file.
 .syntax unified
 
 .globl foo
 foo:
 ldrd r0, r1, foo @ arm_pcrel_10_unscaled
 vldr d0, foo     @ arm_pcrel_10
 adr r2, foo      @ arm_adr_pcrel_12
 ldr r0, foo      @ arm_ldst_pcrel_12
 
+.thumb
+.thumb_func
+
+.globl bar
+bar:
+adr r0, bar      @ thumb_adr_pcrel_10
+adr.w r0, bar    @ t2_adr_pcrel_12
+ldr.w pc, bar    @ t2_ldst_pcrel_12

llvm/test/MC/ARM/thumb1-relax-adr.s

 @ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
 @ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1  | FileCheck --check-prefix=CHECK-ERROR %s
-@ RUN: llvm-mc -triple thumbv6m-none-eabi -filetype=obj %s -o - | llvm-readobj --relocs | FileCheck --check-prefix=CHECK-ELF-T1 %s
-@ RUN: llvm-mc -triple thumbv7m-none-eabi -filetype=obj %s -o - | llvm-readobj --relocs | FileCheck --check-prefix=CHECK-ELF-T2 %s
+@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
 
         .global func1
 _func1:
         adr r0, _func2
 @ CHECK-ERROR: unsupported relocation on symbol
-@ CHECK-ELF-T1: 0x0 R_ARM_THM_PC8 _func2 0x0
-@ CHECK-ELF-T2: 0x0 R_ARM_THM_ALU_PREL_11_0 _func2 0x0

llvm/test/MC/ARM/thumb1-relax-ldrlit.s

 @ RUN: not llvm-mc -triple thumbv6m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
 @ RUN: not llvm-mc -triple thumbv7m-none-macho -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
-@ RUN: llvm-mc -triple thumbv6m-none-eabi -filetype=obj %s -o - | llvm-readobj --relocs | FileCheck --check-prefix=CHECK-ELF-T1 %s
-@ RUN: llvm-mc -triple thumbv7m-none-eabi -filetype=obj %s -o - | llvm-readobj --relocs | FileCheck --check-prefix=CHECK-ELF-T2 %s
+@ RUN: not llvm-mc -triple thumbv7m-none-eabi -filetype=obj -o /dev/null %s 2>&1 | FileCheck --check-prefix=CHECK-ERROR %s
 
         .global func1
 _func1:
         ldr r0, _func2
 @ CHECK-ERROR: unsupported relocation on symbol
-@ CHECK-ELF-T1: 0x0 R_ARM_THM_PC8 _func2 0x0
-@ CHECK-ELF-T2: 0x0 R_ARM_THM_PC12 _func2 0x0