[PATCH] D151863: [x86][MC] Fix movdir64b addressing

Akshay Khadse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 1 02:37:59 PDT 2023


akshaykhadse added a comment.

In D151863#4386528 <https://reviews.llvm.org/D151863#4386528>, @craig.topper wrote:

> MS inline assembly is parsed twice, once by clang and once by the backend.
>
> The issue starts in the clang parsing.
>
> I think the AH comes from this line near the end of `X86AsmParser::CreateMemForMSInlineAsm`
>
>   // Otherwise, we set the base register to a non-zero value
>   // if we don't know the actual value at this time.  This is necessary to
>   // get the matching correct in some cases.
>
> `BaseReg = BaseReg ? BaseReg : 1;`
>
> 1 happens to be the value for AH. This register is a GR64 so it fails the isMem512_GR64 check. This causes the assembly to be considered invalid.
>
> Not sure exactly why it picks a non-zero value. I wonder if we should pick a register other than 1 based on 64bit/32bit/16bit mode?

You are correct. I think it makes sense to pick a value that does not clash with any of the register enum members, but I am not sure if we should have different values for 64bit/32bit/16bit mode.

On a separate note, I wonder if its better to `getMemDisp()->getKind() == llvm::MCExpr::SymbolRef`. This way we don't have to check for `AH` specifically, we just verify if operand is coming from a symbol like `arr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151863



More information about the llvm-commits mailing list