[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