[PATCH] D156505: [AArch64][ELF] Support R_AARCH64_AUTH_ABS64 static relocation

[Daniil Kovalev via Phabricator via llvm-commits]

kovdan01 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCExpr.cpp:201
+  Res = MCValue::get(Res.getSymA(), nullptr, Res.getConstant(), getKind());
+
+  return true;
----------------
MaskRay wrote:
> delete blank line
Done


================
Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCExpr.h:215
+
+  static bool classof(const MCExpr *E) {
+    return E->getKind() == MCExpr::Target;
----------------
MaskRay wrote:
> unneeded?
This is needed for cast operation in `adjustFixupValue` in AArch64AsmBackend.cpp:341. But the check was not correct, fixed that. Decided to use a combination of isa + cast instead of a single dyn_cast since it results in a more elegant one-liner.


================
Comment at: llvm/test/MC/AArch64/ptrauth-elf-reloc.s:94
+
+// RUN: not llvm-mc -triple=aarch64 %t/err1.s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ERR1
----------------
MaskRay wrote:
> llvm-mc has error recovery. You can test all errors in one llvm-mc invocation, instead of using many `err*.s`
Fixed, thanks


================
Comment at: llvm/test/MC/AArch64/ptrauth-elf-reloc.s:95
+// RUN: not llvm-mc -triple=aarch64 %t/err1.s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=ERR1
+
----------------
MaskRay wrote:
> continuation lines are indented by 2 by convention
Fixed, thanks


================
Comment at: llvm/test/MC/AArch64/ptrauth-elf-reloc.s:97
+
+// ERR1: error: expected '('
+
----------------
MaskRay wrote:
> Newer tests are recommended to test the line/column information like:
> 
> ```
> // ERR: :[[#@LINE+1]]:10: error: expected '('
> .quad sym at AUTH)ia,42)
> ```
Applied this, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156505