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

Mon Jan 7 11:53:31 PST 2019

efriedma added inline comments.

================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:223
+        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.
----------------
Indentation.

================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:239
+
+    if (RefKind & AArch64MCExpr::VK_SABS) {
+      switch (AArch64MCExpr::getAddressFrag(RefKind)) {
----------------
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.

================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:274
+
     if (RefKind & AArch64MCExpr::VK_NC)
       Value &= 0xFFFF;
----------------
Please use braces consistently for the whole if-else sequence.

================
Comment at: lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp:405
+      Data[Offset + 3] |= (1 << 6);
+  }
 }
----------------
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.

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

https://reviews.llvm.org/D56029