[PATCH] D80028: [AArch64] Support expression results as immediate values in mov

[Jian Cai via Phabricator via llvm-commits]

jcai19 marked 4 inline comments as done.
jcai19 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:974
+    const MCExpr *E = dyn_cast<MCExpr>(getImm());
+    if (const MCConstantExpr *CE = dyn_cast<MCConstantExpr>(E)) {
+      uint64_t Value = CE->getValue();
----------------
efriedma wrote:
> jcai19 wrote:
> > efriedma wrote:
> > > jcai19 wrote:
> > > > efriedma wrote:
> > > > > What happens when Shift isn't zero?
> > > > Thank you for the comments. And sorry for the delay, I was trying to look into the cases where the value of Shift is not zero. However,  even gcc could not support mov instructions with shift operand.
> > > > 
> > > > $ cat foo.s
> > > > mov x0, 0x1, lsl 16
> > > > 
> > > > $ aarch64-linux-gnu-gcc -c foo.s
> > > > foo.s: Assembler messages:
> > > > foo.s:1: Error: unexpected characters following instruction at operand 2 -- `mov x0,0x1,lsl 16'
> > > > 
> > > > So far the only cases I found with non-zero Shift is during instruction matching stage, when different values of Shift are tried out, and that should not affect this code. Are there additional cases that would cause Shift to not be zero?
> > > This codepath triggers for constructs like "mov x0, 0x10000".
> > Thanks for the example. Since we do not know the result of symbol arithmetic at parsing time so if the result is out of [-0xffff, 0xfff] range, this patch wouldn't be able to handle the cases where Shift is not 0. GNU assembler can handle those cases, and I assume that is because GAS use a multi-iteration design so it can check whether the value is valid after the expression is evaluated, unlike the one-iteration design used by the IAS. I am not sure if there will be many use cases requiring the support of a bigger range. @ostannard previously mentioned
> > 
> > > The GNU assembler also seems to accept expressions which fit into a MOVZ/MOVN with a shift, though I can't see that being very useful for expressions involving symbol addresses.
> > 
> > Maybe this is okay?
> It's probably okay.
> 
> Please make the code here actually verify that Shift is zero, though, instead of just assuming it's zero.
Thanks for the clarification.


================
Comment at: llvm/test/MC/AArch64/mov-invalid-expression-as-immediate.s:9
+// CHECK: mov x0, 1b - 0b
+// CHECK: ^
----------------
efriedma wrote:
> Please also add a testcase for something like `mov x0, 0b`, where the value can't be resolved in the assembler.
This is actually interesting.  GAS assembled the following code 

0:
.skip 4
1:
mov x0, 1b

into 

mov	x0, #0x4 

With the proposed change, IAS would calculate the offset (4) correctly but discard it as it considers the expression 1b unresolved, and generate the following code.

mov	x0, #0x0

What's the expected outcome here? Is subtraction between two symbols the only valid expressions we should accept? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80028