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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I don't have much to add over Eli's existing comments. I think the general form of (symA - symB) + constant may not be handled well for all instructions though.



================
Comment at: lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5313
+  if (!Relocatable || !Res.getSymA() || Res.getSymB())
     return false;
 
----------------
efriedma wrote:
> 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`.
In principle I agree that something like
```
 .text
lab1: nop
lab2: nop
 adr x0, lab2 - lab1 + 0x1000 
```
which would be rejected by the code above, but is equivalent to adr x0, 0x1004 which is.

>From a brief experiment I don't think that the backend can handle expressions like that for all relocations though. I ended up with a RELA relocation with addend 0x1004 to an undefined symbol, which unsurprisingly didn't get past the linker, we'd need an absolute symbol with value 0. May be if the backend were fixed first we could let that one through. 


https://reviews.llvm.org/D51792