[MC] [AArch64] Support resolving signed fixups for :abs_g0_s: etc.

[mgrang]

Summary

This patch is a follow-up to D55896.

[efriedma]

MC/AArch64/fixup-absolute-signed.s

30 ↗

(On Diff #179367)

Can you also add a testcase for something like "movn x0, #:abs_g0_s:onepart_after". (Yes, it's kind of silly, but the assembler allows it.)

[mgrang]

Ping for reviews please.

[mgrang]

Ping2 for reviews please.

[mstorsjo]

In D56029#1343970, @mgrang wrote:

Ping2 for reviews please.

I'd like to defer this one to @efriedma.

[efriedma]

lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

220–221

Indentation.

239

VK_SABS isn't a bit-mask; this technically works because of the specific values of VK_ABS and VK_SABS, but it's very confusing. Please use something like AArch64MCExpr::getSymbolLoc(RefKind) == AArch64MCExpr::VK_SABS instead.

273–274

Please use braces consistently for the whole if-else sequence.

406

We should probably fix getFixupKindInfo() and getFixupKindNumBytes() to correctly account for this bit. Granted, it probably doesn't have much practical effect... I think it'll affect the comments generated by "-show-mc-encoding", and not much else.

[efriedma]

LGTM

lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp

case AArch64::fixup_aarch64_ldst_imm12_scale16:
     // Unsigned 12-bit immediate which gets multiplied by 16
     if (Value >= 0x10000)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     if (Value & 0xf)
       Ctx.reportError(Fixup.getLoc(), "fixup must be 16-byte aligned");
     return Value >> 4;
   case AArch64::fixup_aarch64_movw: {
     AArch64MCExpr::VariantKind RefKind =
         static_cast<AArch64MCExpr::VariantKind>(Target.getRefKind());
-    if (AArch64MCExpr::getSymbolLoc(RefKind) != AArch64MCExpr::VK_ABS) {
-      if (AArch64MCExpr::getSymbolLoc(RefKind) != AArch64MCExpr::VK_SABS) {
-        // VK_GOTTPREL, VK_TPREL, VK_DTPREL are movw fixups, but they can't
-        // ever be resolved in the assembler.
-        Ctx.reportError(Fixup.getLoc(),
-                        "relocation for a thread-local variable points to an "
-                        "absolute symbol");
-        return Value;
-      }
-      Ctx.reportError(Fixup.getLoc(),
-                      "resolvable R_AARCH64_MOVW_SABS_G* fixups are not "
-                       "yet implemented");
-      return Value;
-    }
+    if (AArch64MCExpr::getSymbolLoc(RefKind) != AArch64MCExpr::VK_ABS &&
+        AArch64MCExpr::getSymbolLoc(RefKind) != AArch64MCExpr::VK_SABS) {
+
     if (!IsResolved) {
       // FIXME: Figure out when this can actually happen, and verify our
       // behavior.
       Ctx.reportError(Fixup.getLoc(), "unresolved movw fixup not yet "
                                       "implemented");
       return Value;
     }
+
+    if (AArch64MCExpr::getSymbolLoc(RefKind) == AArch64MCExpr::VK_SABS) {
+      switch (AArch64MCExpr::getAddressFrag(RefKind)) {
+      case AArch64MCExpr::VK_G0:
+        break;
+      case AArch64MCExpr::VK_G1:
+        SignedValue = SignedValue >> 16;
+        break;
+      case AArch64MCExpr::VK_G2:
+        SignedValue = SignedValue >> 32;
+        break;
+      case AArch64MCExpr::VK_G3:
+        SignedValue = SignedValue >> 48;
+        break;
+      default:
+        llvm_unreachable("Variant kind doesn't correspond to fixup");
+      }
+
+    } else {
       switch (AArch64MCExpr::getAddressFrag(RefKind)) {
       case AArch64MCExpr::VK_G0:
         break;
       case AArch64MCExpr::VK_G1:
         Value = Value >> 16;
         break;
       case AArch64MCExpr::VK_G2:
         Value = Value >> 32;
         break;
       case AArch64MCExpr::VK_G3:
         Value = Value >> 48;
         break;
       default:
         llvm_unreachable("Variant kind doesn't correspond to fixup");
       }
-    if (RefKind & AArch64MCExpr::VK_NC)
+    }
+
+    if (RefKind & AArch64MCExpr::VK_NC) {
       Value &= 0xFFFF;
-    else if (Value > 0xFFFF)
+    }
+    else if (RefKind & AArch64MCExpr::VK_SABS) {
+      if (SignedValue > 0xFFFF || SignedValue < -0xFFFF)
+        Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
+
+      // Invert the negative immediate because it will feed into a MOVN.
+      if (SignedValue < 0)
+        SignedValue = ~SignedValue;
+      Value = static_cast<uint64_t>(SignedValue);
+    }
+    else if (Value > 0xFFFF) {
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
+    }
     return Value;
   }
   case AArch64::fixup_aarch64_pcrel_branch14:
     // Signed 16-bit immediate
     if (SignedValue > 32767 || SignedValue < -32768)
       Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     // Low two bits are not encoded (4-byte alignment assumed).
     if (Value & 0x3)
void AArch64AsmBackend::applyFixup(const MCAssembler &Asm, const MCFixup &Fixup,
                                    MutableArrayRef<char> Data, uint64_t Value,
                                    bool IsResolved,
                                    const MCSubtargetInfo *STI) const {
   unsigned NumBytes = getFixupKindNumBytes(Fixup.getKind());
   if (!Value)
     return; // Doesn't change encoding.
   MCFixupKindInfo Info = getFixupKindInfo(Fixup.getKind());
   MCContext &Ctx = Asm.getContext();
+  int64_t SignedValue = static_cast<int64_t>(Value);
   // Apply any target-specific value adjustments.
   Value = adjustFixupValue(Fixup, Target, Value, Ctx, TheTriple, IsResolved);
 
   // Shift the value into position.
   Value <<= Info.TargetOffset;
 
   unsigned Offset = Fixup.getOffset();
   assert(Offset + NumBytes <= Data.size() && "Invalid fixup offset!");
if (FulleSizeInBytes == 0) {
     // Handle as big-endian
     assert((Offset + FulleSizeInBytes) <= Data.size() && "Invalid fixup size!");
     assert(NumBytes <= FulleSizeInBytes && "Invalid fixup size!");
     for (unsigned i = 0; i != NumBytes; ++i) {
       unsigned Idx = FulleSizeInBytes - 1 - i;
       Data[Offset + Idx] |= uint8_t((Value >> (i * 8)) & 0xff);
     }
   }
+
+  // FIXME: getFixupKindInfo() and getFixupKindNumBytes() could be fixed to
+  // handle this more cleanly. This may affect the output of -show-mc-encoding.
+  AArch64MCExpr::VariantKind RefKind =
+    static_cast<AArch64MCExpr::VariantKind>(Target.getRefKind());
+  if (RefKind & AArch64MCExpr::VK_SABS) {
+    // If the immediate is negative, generate MOVN else MOVZ.
+    // (Bit 30 = 0) ==> MOVN, (Bit 30 = 1) ==> MOVZ.
+    if (SignedValue < 0)
+      Data[Offset + 3] &= ~(1 << 6);
+    else
+      Data[Offset + 3] |= (1 << 6);
+  }
 }
 
 bool AArch64AsmBackend::mayNeedRelaxation(const MCInst &Inst,
                                           const MCSubtargetInfo &STI) const {
   return false;
 }
 
 bool AArch64AsmBackend::fixupNeedsRelaxation(const MCFixup &Fixup,

test/MC/AArch64/fixup-absolute-signed.s

+// RUN: llvm-mc -triple aarch64--none-eabi -filetype obj < %s -o - | llvm-objdump -d - | FileCheck %s
+
+onepart_before = 12345
+twopart_before = -12345678
+threepart_before = -1234567890
+
+// CHECK: movn     x0, #0, lsl #32
+// CHECK: movn     x0, #0, lsl #32
+movz x0, #:abs_g2_s:threepart_before
+movz x0, #:abs_g2_s:threepart_after
+
+// CHECK: movk    x0, #65535, lsl #32
+// CHECK: movk    x0, #65535, lsl #32
+movk x0, #:abs_g2_nc:threepart_before
+movk x0, #:abs_g2_nc:threepart_after
+
+// CHECK: mov     x0, #-12320769
+// CHECK: mov     x0, #-12320769
+movz x0, #:abs_g1_s:twopart_before
+movz x0, #:abs_g1_s:twopart_after
+
+// CHECK: movk    x0, #46697, lsl #16
+// CHECK: movk    x0, #46697, lsl #16
+movk x0, #:abs_g1_nc:threepart_before
+movk x0, #:abs_g1_nc:threepart_after
+
+// CHECK: mov     x0, #12345
+// CHECK: mov     x0, #12345
+movz x0, #:abs_g0_s:onepart_before
+movz x0, #:abs_g0_s:onepart_after
+
+// CHECK: movk    x0, #64814
+// CHECK: movk    x0, #64814
+movk x0, #:abs_g0_nc:threepart_before
+movk x0, #:abs_g0_nc:threepart_after
+
+// CHECK: mov     x0, #12345
+// CHECK: mov     x0, #12345
+movn x0, #:abs_g0_s:onepart_before
+movn x0, #:abs_g0_s:onepart_after
+
+onepart_after = 12345
+twopart_after = -12345678
+threepart_after = -1234567890

test/MC/AArch64/fixup-out-of-range.s

// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup not sufficiently aligned
   b unaligned
 
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
   movz x0, #:abs_g0:value1
 
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
   movz x0, #:abs_g1:value2
 
-// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: resolvable R_AARCH64_MOVW_SABS_G* fixups are not yet implemented
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
   movz x0, #:abs_g0_s:value1
 
+// CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: fixup value out of range
+  movz x0, #:abs_g1_s:value2
+
 // CHECK: :[[@LINE+1]]:{{[0-9]+}}: error: relocation for a thread-local variable points to an absolute symbol
   movz x0, #:tprel_g0:value1
 
 
   .byte 0
 unaligned:
   .byte 0
 
   .space 1<<27
   .balign 8
 distant:
   .word 0
 value1 = 0x12345678
 value2 = 0x123456789