[PATCH] D149579: [X86][MC] Fix parsing Intel syntax indirect branch with symbol only

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D149579#4312025 <https://reviews.llvm.org/D149579#4312025>, @alvinhochun wrote:

> Turns out this change affects MS inline assembly and possibly other inline assemblies as well. Looking at ms-inline-asm-functions.c <https://github.com/llvm/llvm-project/blob/7cf7ebd9170119f61ff998b61fe0de261dcdae65/clang/test/CodeGen/ms-inline-asm-functions.c> (on Compiler Explorer - https://godbolt.org/z/3xfW75Go4):
>
> - Direct call: `__asm call k;` generates the inline assembly `call dword ptr ${1:P}` (which seems wrong, should be `call ${1:P}` instead?), which is converted into `call dword ptr _k`, gets compiled as `call dword ptr [_k]` with this patch (incorrect).
> - Call through a global function pointer, which is marked as a "broken case" in the test: `__asm call kptr;` generates the inline assembly `call $4`, which is converted into `call [offset _kptr]`, gets compiled as `call dword ptr [_kptr]` with this patch, which actually fixes the test case, //except that// `call [offset _kptr]` doesn't seem right (see below).
> - Direct jump: `__asm jmp k;` generates the inline assembly `jmp dword ptr ${1:P}`, same issue as direct call.
>
> Among these, the usage of operator `offset` in `call` seems questionable. I checked the behaviour of other assemblers:
>
> - NASM:
>   - Does not support `offset` operator.
> - MASM (ml) Version 14.35.32215.0:
>   - `call offset fn` is oddly assembled as `push cs \n call fn`.
>   - `call [offset fn]` generates error A2023:instruction operand must have size.
>   - `call dword ptr [offset fn]` is assembled as `call dword ptr [fn]`.
> - GAS (Intel syntax):
>   - Both `call offset fn` and `call [offset fn]` are assembled the same way as `call fn`, which seems odd.
>   - `call dword ptr [offset fn]` is assembled as `call dword ptr [fn]`.
>
> ---
>
> I think this issue may end up being too much for me to handle. I already spent more time than I wanted tracing the code and trying various hacks, but couldn't quite come up with proper fixes. Will someone be willing to take over this?

OK. I  think after D149695 <https://reviews.llvm.org/D149695> (landed), D149920 <https://reviews.llvm.org/D149920>, and this patch D149579 <https://reviews.llvm.org/D149579>, these cases should be correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149579