[PATCH] D51792: [AArch64] Attempt to parse expressions as adr/adrp operands

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5289
 bool
 AArch64AsmParser::classifySymbolRef(const MCExpr *Expr,
                                     AArch64MCExpr::VariantKind &ELFRefKind,
----------------
classifySymbolRef is used for other instructions, including movw/movk, add/sub, and ldr.  Please make sure we have appropriate test coverage.

It looks like a lot of instructions try to do some sort of fallback if classifySymbolRef fails.  I'm not sure that actually make sense, but please make sure we have test coverage, at least.


================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5313
+  if (!Relocatable || !Res.getSymA() || Res.getSymB())
     return false;
 
----------------
It looks like there isn't any test coverage for the `!Res.getSymA() || Res.getSymB()` part?  In particular, I'm not sure why you'd want to reject an absolute value, in general.

There should probably also be test coverage that someone doesn't write something silly like `adr x0, :lo12:f`.


================
Comment at: test/MC/AArch64/adr-badsyms.s:36
+// CHECK-NEXT:   ^
\ No newline at end of file

----------------
Newline?


https://reviews.llvm.org/D51792