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

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 23 07:42:54 PDT 2018


thakis added a comment.

In https://reviews.llvm.org/D37461#1150631, @RKSimon wrote:

> @coby Are you still looking as this? If not (@coby hasn't been active on phab since 2017) @thakis would you be in a position to commandeer this and complete it?


I can try, but I don't know this code well. I've uploaded a rebased but otherwise unmodified version at https://reviews.llvm.org/D49648

The crash I posted above is because IntelExpr() used to map a Scale of 0 to 1. If I add that back in, my crash goes away.

However, offset is also supposed to work with quoted symbols. If I allow that like so:

  +  if (!isParsingInlineAsm()) {
  +    if ((getTok().isNot(AsmToken::Identifier) &&
  +         getTok().isNot(AsmToken::String)) ||
  +        getParser().parsePrimaryExpr(Val, End))
  +      return Error(Start, "unexpected token!");

then one of the two examples in PR36676 starts working, but the other asserts due to an unknown relocation. The problem is that offset is modeled as a `X86Operand::CreateImm()` with a fixup, but since intel asm instrs don't have sizes the logic sees that the imm is 0 and hence creates a 16-bit imm and we then get a 2 byte reloc, which aren't implemented. This is wrong anyhow since `push offset foo` should create a pointer size relocation. `X86AsmParser::ParseIntelOperand()` could not create an imm but a memory ref if `SM.isOffsetOperator()` and that makes things go, but then the push is encoded with a PUSH32rmm instead of a PUSH32i8, which is a byte larger and different from what clang-cl writes if you don't pass /FA, so I'd like to fix that too. I haven't yet figured out how :-)


Repository:
  rL LLVM

https://reviews.llvm.org/D37461





More information about the llvm-commits mailing list