[PATCH] D37461: [X86][AsmParser] re-introduce 'offset' operator

Thu Dec 12 13:59:06 PST 2019

epastor added a comment.

In D37461#1383242 <https://reviews.llvm.org/D37461#1383242>, @thakis wrote:

> There are at least 3 things missing to get `check-clang` to work:
>
> 1. clang currently supports `offset localvar`, which cl.exe doesn't support. This is currently rewritten to the stack address; after this patch it instead becomes `offset localvar` but localvar isn't an existing symbol of course. So for locals, this would still have to do the rewrite.
> 2. For `offset Foo::bar`, clang used to rewrite that to a local symbol, not it just prints `offset Foo::bar` – I think we probably need to put the mangled name there instead.
> 3. For `  __asm mov [eax], offset var` we get `ambiguous operand size for instruction 'mov'` now; that needs fixing.

I've got a local draft that fixes (1) and (2), handling both as operands. It avoids having to special-case local symbols too deeply, handling everything as an operand - local symbols have their location on the stack passed in through a register (and so can't be included in compound expressions), global symbols come in as an immediate (and so can be included in compound expressions).

(3) is interesting. Looking at https://godbolt.org/z/QQ9ASx, currently clang rewrites `offset x` to an explicitly pointer-sized register, with the choice of register hard-wired to RBX/EBX. Now that I'm handling local symbol offset as an operand, we get operand size ambiguity... which seems appropriate to me. Would it be reasonable to consider this the new correct behavior, or do we need to preserve it as is?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D37461