[PATCH] D110267: [InlineAsm, SystemZ] Handle inline assembly address operands.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

Finally getting back to this patch - sorry for the long delay.

I think it would be worthwhile to explore using a dedicated `C_Address` constraint type - these constraints __are__ different from memory.   In particular, memory constraints usually require the front-end to also use the "indirect" flag since the asm statement requires the **address** as argument.   For actual address operands, these are already addresses at the source level, so they are not indirect.

Using different `C_Address` and `C_Memory` types would also force all places that treat constaints to be explicitly updated to handle address constraints, requiring deliberate choices.  This seems preferable since the constraint types frequently do require different treatment.

As an aside, in addition to `SelectionDAGBuilder`, which this patch updates, there is also inline asm constraint handling in `GlobalISel/InlineAsmLowering` which needs similar changes.

Finally, I tried to play with address constraints on Intel, and the `"p"` constraint as currently implemented on Intel seems to be actually incorrect.   For example, compiling the following code:

  void *test (void *ptr)
  {
          void *ret;
          asm ("lea %1, %0" : "=r" (ret) : "p" (ptr));
          return ret;
  }

with GCC results in:

  lea %rdi, %rax
  ret

while with clang we get instead:

  movq    %rdi, -8(%rsp)
  leaq    -8(%rsp), %rax
  retq

(i.e. we get the address of a stack slot where `ptr` was stored, rather than the value of `ptr`)

So it might make sense to have two patches instead, a first patch that introduces the `C_Address` mechanism, and uses it to implement the `"p"` constraint in a platform-independent manner, fixing in particular this bug on Intel.  And then in a second patch add the SystemZ-specific address constraints.


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

https://reviews.llvm.org/D110267