[llvm] r277402 - Revert r276895 "[MC][X86] Fix Intel Operand assembly parsing for .set ids"

Bruno Cardoso Lopes via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 20:02:18 PDT 2016


Thanks Hans!

On Mon, Aug 1, 2016 at 4:42 PM, Hans Wennborg <hans at chromium.org> wrote:
> r277405 should hopefully do it.
>
> Sorry for the breakage.
>
>  - Hans
>
> On Mon, Aug 1, 2016 at 4:33 PM, Hans Wennborg <hans at chromium.org> wrote:
>> Looking now. I didn't realize this would break Clang stuff..
>>
>> On Mon, Aug 1, 2016 at 4:30 PM, Bruno Cardoso Lopes via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> Hi Hans,
>>>
>>> The revert broke
>>> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/26788
>>> Any chance there's a follow up to the original commit that didn't get
>>> reverted as well?
>>>
>>> On Mon, Aug 1, 2016 at 4:00 PM, Hans Wennborg via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>> Author: hans
>>>> Date: Mon Aug  1 18:00:01 2016
>>>> New Revision: 277402
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=277402&view=rev
>>>> Log:
>>>> Revert r276895 "[MC][X86] Fix Intel Operand assembly parsing for .set ids"
>>>>
>>>> This caused PR28805. Adding a regression test.
>>>>
>>>> Added:
>>>>     llvm/trunk/test/MC/AsmParser/pr28805.ll
>>>> Modified:
>>>>     llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
>>>>     llvm/trunk/test/MC/X86/intel-syntax-encoding.s
>>>>     llvm/trunk/test/MC/X86/intel-syntax-error.s
>>>>
>>>> Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp?rev=277402&r1=277401&r2=277402&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
>>>> +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Mon Aug  1 18:00:01 2016
>>>> @@ -698,11 +698,14 @@ private:
>>>>    std::unique_ptr<X86Operand> ParseIntelOperator(unsigned OpKind);
>>>>    std::unique_ptr<X86Operand>
>>>>    ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
>>>> +  std::unique_ptr<X86Operand>
>>>> +  ParseIntelMemOperand(int64_t ImmDisp, SMLoc StartLoc, unsigned Size);
>>>>    std::unique_ptr<X86Operand> ParseRoundingModeOp(SMLoc Start, SMLoc End);
>>>>    bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
>>>> -  std::unique_ptr<X86Operand>
>>>> -  ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
>>>> -                           bool isSymbol, unsigned Size);
>>>> +  std::unique_ptr<X86Operand> ParseIntelBracExpression(unsigned SegReg,
>>>> +                                                       SMLoc Start,
>>>> +                                                       int64_t ImmDisp,
>>>> +                                                       unsigned Size);
>>>>    bool ParseIntelIdentifier(const MCExpr *&Val, StringRef &Identifier,
>>>>                              InlineAsmIdentifierInfo &Info,
>>>>                              bool IsUnevaluatedOperand, SMLoc &End);
>>>> @@ -1383,8 +1386,7 @@ bool X86AsmParser::ParseIntelExpression(
>>>>
>>>>  std::unique_ptr<X86Operand>
>>>>  X86AsmParser::ParseIntelBracExpression(unsigned SegReg, SMLoc Start,
>>>> -                                       int64_t ImmDisp, bool isSymbol,
>>>> -                                       unsigned Size) {
>>>> +                                       int64_t ImmDisp, unsigned Size) {
>>>>    MCAsmParser &Parser = getParser();
>>>>    const AsmToken &Tok = Parser.getTok();
>>>>    SMLoc BracLoc = Tok.getLoc(), End = Tok.getEndLoc();
>>>> @@ -1434,21 +1436,6 @@ X86AsmParser::ParseIntelBracExpression(u
>>>>      Disp = NewDisp;
>>>>    }
>>>>
>>>> -  if (isSymbol) {
>>>> -    if (SM.getSym()) {
>>>> -      Error(Start, "cannot use more than one symbol in memory operand");
>>>> -      return nullptr;
>>>> -    }
>>>> -    if (SM.getBaseReg()) {
>>>> -      Error(Start, "cannot use base register with variable reference");
>>>> -      return nullptr;
>>>> -    }
>>>> -    if (SM.getIndexReg()) {
>>>> -      Error(Start, "cannot use index register with variable reference");
>>>> -      return nullptr;
>>>> -    }
>>>> -  }
>>>> -
>>>>    int BaseReg = SM.getBaseReg();
>>>>    int IndexReg = SM.getIndexReg();
>>>>    int Scale = SM.getScale();
>>>> @@ -1554,7 +1541,7 @@ X86AsmParser::ParseIntelSegmentOverride(
>>>>    }
>>>>
>>>>    if (getLexer().is(AsmToken::LBrac))
>>>> -    return ParseIntelBracExpression(SegReg, Start, ImmDisp, false, Size);
>>>> +    return ParseIntelBracExpression(SegReg, Start, ImmDisp, Size);
>>>>
>>>>    const MCExpr *Val;
>>>>    SMLoc End;
>>>> @@ -1611,6 +1598,66 @@ X86AsmParser::ParseRoundingModeOp(SMLoc
>>>>    }
>>>>    return ErrorOperand(Tok.getLoc(), "unknown token in expression");
>>>>  }
>>>> +/// ParseIntelMemOperand - Parse intel style memory operand.
>>>> +std::unique_ptr<X86Operand> X86AsmParser::ParseIntelMemOperand(int64_t ImmDisp,
>>>> +                                                               SMLoc Start,
>>>> +                                                               unsigned Size) {
>>>> +  MCAsmParser &Parser = getParser();
>>>> +  const AsmToken &Tok = Parser.getTok();
>>>> +  SMLoc End;
>>>> +
>>>> +  // Parse ImmDisp [ BaseReg + Scale*IndexReg + Disp ].
>>>> +  if (getLexer().is(AsmToken::LBrac))
>>>> +    return ParseIntelBracExpression(/*SegReg=*/0, Start, ImmDisp, Size);
>>>> +  assert(ImmDisp == 0);
>>>> +
>>>> +  const MCExpr *Val;
>>>> +  if (!isParsingInlineAsm()) {
>>>> +    if (getParser().parsePrimaryExpr(Val, End))
>>>> +      return ErrorOperand(Tok.getLoc(), "unknown token in expression");
>>>> +
>>>> +    return X86Operand::CreateMem(getPointerWidth(), Val, Start, End, Size);
>>>> +  }
>>>> +
>>>> +  InlineAsmIdentifierInfo Info;
>>>> +  StringRef Identifier = Tok.getString();
>>>> +  if (ParseIntelIdentifier(Val, Identifier, Info,
>>>> +                           /*Unevaluated=*/false, End))
>>>> +    return nullptr;
>>>> +
>>>> +  if (!getLexer().is(AsmToken::LBrac))
>>>> +    return CreateMemForInlineAsm(/*SegReg=*/0, Val, /*BaseReg=*/0, /*IndexReg=*/0,
>>>> +                                 /*Scale=*/1, Start, End, Size, Identifier, Info);
>>>> +
>>>> +  Parser.Lex(); // Eat '['
>>>> +
>>>> +  // Parse Identifier [ ImmDisp ]
>>>> +  IntelExprStateMachine SM(/*ImmDisp=*/0, /*StopOnLBrac=*/true,
>>>> +                           /*AddImmPrefix=*/false);
>>>> +  if (ParseIntelExpression(SM, End))
>>>> +    return nullptr;
>>>> +
>>>> +  if (SM.getSym()) {
>>>> +    Error(Start, "cannot use more than one symbol in memory operand");
>>>> +    return nullptr;
>>>> +  }
>>>> +  if (SM.getBaseReg()) {
>>>> +    Error(Start, "cannot use base register with variable reference");
>>>> +    return nullptr;
>>>> +  }
>>>> +  if (SM.getIndexReg()) {
>>>> +    Error(Start, "cannot use index register with variable reference");
>>>> +    return nullptr;
>>>> +  }
>>>> +
>>>> +  const MCExpr *Disp = MCConstantExpr::create(SM.getImm(), getContext());
>>>> +  // BaseReg is non-zero to avoid assertions.  In the context of inline asm,
>>>> +  // we're pointing to a local variable in memory, so the base register is
>>>> +  // really the frame or stack pointer.
>>>> +  return X86Operand::CreateMem(getPointerWidth(), /*SegReg=*/0, Disp,
>>>> +                               /*BaseReg=*/1, /*IndexReg=*/0, /*Scale=*/1,
>>>> +                               Start, End, Size, Identifier, Info.OpDecl);
>>>> +}
>>>>
>>>>  /// Parse the '.' operator.
>>>>  bool X86AsmParser::ParseIntelDotOperator(const MCExpr *Disp,
>>>> @@ -1757,9 +1804,50 @@ std::unique_ptr<X86Operand> X86AsmParser
>>>>      Parser.Lex(); // Eat ptr.
>>>>      PtrInOperand = true;
>>>>    }
>>>> -
>>>>    Start = Tok.getLoc();
>>>>
>>>> +  // Immediate.
>>>> +  if (getLexer().is(AsmToken::Integer) || getLexer().is(AsmToken::Minus) ||
>>>> +      getLexer().is(AsmToken::Tilde) || getLexer().is(AsmToken::LParen)) {
>>>> +    AsmToken StartTok = Tok;
>>>> +    IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
>>>> +                             /*AddImmPrefix=*/false);
>>>> +    if (ParseIntelExpression(SM, End))
>>>> +      return nullptr;
>>>> +
>>>> +    int64_t Imm = SM.getImm();
>>>> +    if (isParsingInlineAsm()) {
>>>> +      unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
>>>> +      if (StartTok.getString().size() == Len)
>>>> +        // Just add a prefix if this wasn't a complex immediate expression.
>>>> +        InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
>>>> +      else
>>>> +        // Otherwise, rewrite the complex expression as a single immediate.
>>>> +        InstInfo->AsmRewrites->emplace_back(AOK_Imm, Start, Len, Imm);
>>>> +    }
>>>> +
>>>> +    if (getLexer().isNot(AsmToken::LBrac)) {
>>>> +      // If a directional label (ie. 1f or 2b) was parsed above from
>>>> +      // ParseIntelExpression() then SM.getSym() was set to a pointer to
>>>> +      // to the MCExpr with the directional local symbol and this is a
>>>> +      // memory operand not an immediate operand.
>>>> +      if (SM.getSym())
>>>> +        return X86Operand::CreateMem(getPointerWidth(), SM.getSym(), Start, End,
>>>> +                                     Size);
>>>> +
>>>> +      const MCExpr *ImmExpr = MCConstantExpr::create(Imm, getContext());
>>>> +      return X86Operand::CreateImm(ImmExpr, Start, End);
>>>> +    }
>>>> +
>>>> +    // Only positive immediates are valid.
>>>> +    if (Imm < 0)
>>>> +      return ErrorOperand(Start, "expected a positive immediate displacement "
>>>> +                          "before bracketed expr.");
>>>> +
>>>> +    // Parse ImmDisp [ BaseReg + Scale*IndexReg + Disp ].
>>>> +    return ParseIntelMemOperand(Imm, Start, Size);
>>>> +  }
>>>> +
>>>>    // rounding mode token
>>>>    if (getSTI().getFeatureBits()[X86::FeatureAVX512] &&
>>>>        getLexer().is(AsmToken::LCurly))
>>>> @@ -1767,8 +1855,7 @@ std::unique_ptr<X86Operand> X86AsmParser
>>>>
>>>>    // Register.
>>>>    unsigned RegNo = 0;
>>>> -  if (getLexer().is(AsmToken::Identifier) &&
>>>> -      !ParseRegister(RegNo, Start, End)) {
>>>> +  if (!ParseRegister(RegNo, Start, End)) {
>>>>      // If this is a segment register followed by a ':', then this is the start
>>>>      // of a segment override, otherwise this is a normal register reference.
>>>>      // In case it is a normal register and there is ptr in the operand this
>>>> @@ -1780,63 +1867,12 @@ std::unique_ptr<X86Operand> X86AsmParser
>>>>        }
>>>>        return X86Operand::CreateReg(RegNo, Start, End);
>>>>      }
>>>> +
>>>>      return ParseIntelSegmentOverride(/*SegReg=*/RegNo, Start, Size);
>>>>    }
>>>>
>>>> -  // Immediates and Memory
>>>> -
>>>> -  // Parse [ BaseReg + Scale*IndexReg + Disp ].
>>>> -  if (getLexer().is(AsmToken::LBrac))
>>>> -    return ParseIntelBracExpression(/*SegReg=*/0, Start, /*ImmDisp=*/0, false,
>>>> -                                    Size);
>>>> -
>>>> -  AsmToken StartTok = Tok;
>>>> -  IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
>>>> -                           /*AddImmPrefix=*/false);
>>>> -  if (ParseIntelExpression(SM, End))
>>>> -    return nullptr;
>>>> -
>>>> -  bool isSymbol = SM.getSym() && SM.getSym()->getKind() != MCExpr::Constant;
>>>> -  int64_t Imm = SM.getImm();
>>>> -  if (SM.getSym() && SM.getSym()->getKind() == MCExpr::Constant)
>>>> -    SM.getSym()->evaluateAsAbsolute(Imm);
>>>> -
>>>> -  if (StartTok.isNot(AsmToken::Identifier) &&
>>>> -      StartTok.isNot(AsmToken::String) && isParsingInlineAsm()) {
>>>> -    unsigned Len = Tok.getLoc().getPointer() - Start.getPointer();
>>>> -    if (StartTok.getString().size() == Len)
>>>> -      // Just add a prefix if this wasn't a complex immediate expression.
>>>> -      InstInfo->AsmRewrites->emplace_back(AOK_ImmPrefix, Start);
>>>> -    else
>>>> -      // Otherwise, rewrite the complex expression as a single immediate.
>>>> -      InstInfo->AsmRewrites->emplace_back(AOK_Imm, Start, Len, Imm);
>>>> -  }
>>>> -
>>>> -  if (getLexer().isNot(AsmToken::LBrac)) {
>>>> -    // If a directional label (ie. 1f or 2b) was parsed above from
>>>> -    // ParseIntelExpression() then SM.getSym() was set to a pointer to
>>>> -    // to the MCExpr with the directional local symbol and this is a
>>>> -    // memory operand not an immediate operand.
>>>> -    if (isSymbol) {
>>>> -      if (isParsingInlineAsm())
>>>> -        return CreateMemForInlineAsm(/*SegReg=*/0, SM.getSym(), /*BaseReg=*/0,
>>>> -                                     /*IndexReg=*/0,
>>>> -                                     /*Scale=*/1, Start, End, Size,
>>>> -                                     SM.getSymName(), SM.getIdentifierInfo());
>>>> -      return X86Operand::CreateMem(getPointerWidth(), SM.getSym(), Start, End,
>>>> -                                   Size);
>>>> -    }
>>>> -
>>>> -    const MCExpr *ImmExpr = MCConstantExpr::create(Imm, getContext());
>>>> -    return X86Operand::CreateImm(ImmExpr, Start, End);
>>>> -  }
>>>> -
>>>> -  // Only positive immediates are valid.
>>>> -  if (Imm < 0)
>>>> -    return ErrorOperand(Start, "expected a positive immediate displacement "
>>>> -                               "before bracketed expr.");
>>>> -
>>>> -  return ParseIntelBracExpression(/*SegReg=*/0, Start, Imm, isSymbol, Size);
>>>> +  // Memory operand.
>>>> +  return ParseIntelMemOperand(/*Disp=*/0, Start, Size);
>>>>  }
>>>>
>>>>  std::unique_ptr<X86Operand> X86AsmParser::ParseATTOperand() {
>>>> @@ -1880,7 +1916,7 @@ std::unique_ptr<X86Operand> X86AsmParser
>>>>      SMLoc Start = Parser.getTok().getLoc(), End;
>>>>      if (getSTI().getFeatureBits()[X86::FeatureAVX512])
>>>>        return ParseRoundingModeOp(Start, End);
>>>> -    return ErrorOperand(Start, "Unexpected '{' in expression");
>>>> +    return ErrorOperand(Start, "unknown token in expression");
>>>>    }
>>>>    }
>>>>  }
>>>>
>>>> Added: llvm/trunk/test/MC/AsmParser/pr28805.ll
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/AsmParser/pr28805.ll?rev=277402&view=auto
>>>> ==============================================================================
>>>> --- llvm/trunk/test/MC/AsmParser/pr28805.ll (added)
>>>> +++ llvm/trunk/test/MC/AsmParser/pr28805.ll Mon Aug  1 18:00:01 2016
>>>> @@ -0,0 +1,20 @@
>>>> +; RUN: llc %s -o - | FileCheck %s
>>>> +
>>>> +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
>>>> +target triple = "x86_64-pc-windows-msvc18.0.31101"
>>>> +
>>>> +define i32 @_xbegin() {
>>>> +entry:
>>>> +  %res = alloca i32, align 4
>>>> +  %0 = bitcast i32* %res to i8*
>>>> +  store i32 -1, i32* %res, align 4
>>>> +  call void asm sideeffect inteldialect ".byte 0xC7\0A\09.byte 0xF8\0A\09.byte 2\0A\09.byte 0\0A\09.byte 0\0A\09.byte 0\0A\09jmp .L__MSASMLABEL_.0__L2\0A\09mov dword ptr $0, eax\0A\09.L__MSASMLABEL_.0__L2:", "=*m,~{dirflag},~{fpsr},~{flags}"(i32* nonnull %res)
>>>> +  %1 = load i32, i32* %res, align 4
>>>> +  ret i32 %1
>>>> +}
>>>> +
>>>> +; CHECK-NOT: Error parsing inline asm
>>>> +
>>>> +; CHECK-LABEL: _xbegin:
>>>> +; CHECK: jmp .L__MSASMLABEL_.0__L2
>>>> +; CHECK: .L__MSASMLABEL_.0__L2:
>>>>
>>>> Modified: llvm/trunk/test/MC/X86/intel-syntax-encoding.s
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax-encoding.s?rev=277402&r1=277401&r2=277402&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/MC/X86/intel-syntax-encoding.s (original)
>>>> +++ llvm/trunk/test/MC/X86/intel-syntax-encoding.s Mon Aug  1 18:00:01 2016
>>>> @@ -76,8 +76,3 @@ LBB0_3:
>>>>  // CHECK: encoding: [0xca,0x08,0x00]
>>>>      retf 8
>>>>
>>>> -    .set FOO, 2
>>>> -    cmp eax, FOO
>>>> -// CHECK: encoding: [0x83,0xf8,0x02]
>>>> -    cmp eax, FOO[eax]
>>>> -// CHECK: encoding: [0x67,0x3b,0x40,0x02]
>>>>
>>>> Modified: llvm/trunk/test/MC/X86/intel-syntax-error.s
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/intel-syntax-error.s?rev=277402&r1=277401&r2=277402&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/test/MC/X86/intel-syntax-error.s (original)
>>>> +++ llvm/trunk/test/MC/X86/intel-syntax-error.s Mon Aug  1 18:00:01 2016
>>>> @@ -11,16 +11,3 @@ _test2:
>>>>  .att_syntax noprefix
>>>>  // CHECK: error: '.att_syntax noprefix' is not supported: registers must have a '%' prefix in .att_syntax
>>>>         movl    $257, -4(esp)
>>>> -
>>>> -
>>>> -.intel_syntax noprefix
>>>> -
>>>> -.global arr
>>>> -.global i
>>>> -.set FOO, 2
>>>> -//CHECK-STDERR: error: cannot use base register with variable reference
>>>> -mov eax, DWORD PTR arr[ebp + 1 + (2 * 5) - 3 + 1<<1]
>>>> -//CHECK-STDERR: error: cannot use index register with variable reference
>>>> -mov eax, DWORD PTR arr[esi*4]
>>>> -//CHECK-STDERR: error: cannot use more than one symbol in memory operand
>>>> -mov eax, DWORD PTR arr[i]
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>> --
>>> Bruno Cardoso Lopes
>>> http://www.brunocardoso.cc
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc


More information about the llvm-commits mailing list