[llvm] r339895 - [MC][X86] Enhance X86 Register expression handling to more closely match GCC.

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 21 12:58:51 PDT 2018


Thanks! Merged in r340329.

On Tue, Aug 21, 2018 at 10:59 AM, Nirav Davé <niravd at google.com> wrote:
> It seems reasonble to me. We should also commit d0k's followup fix
> (r339896).
>
> -Nirav
>
> On Tue, Aug 21, 2018 at 1:15 PM, Reid Kleckner <rnk at google.com> wrote:
>>
>> Sure.
>>
>> On Tue, Aug 21, 2018 at 9:58 AM Hans Wennborg <hans at chromium.org> wrote:
>>>
>>> Someone asked on IRC about merging this to 7.0. What do you think?
>>>
>>> On Thu, Aug 16, 2018 at 9:31 AM, Nirav Dave via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>> > Author: niravd
>>> > Date: Thu Aug 16 09:31:14 2018
>>> > New Revision: 339895
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=339895&view=rev
>>> > Log:
>>> > [MC][X86] Enhance X86 Register expression handling to more closely
>>> > match GCC.
>>> >
>>> > Allow the comparison of x86 registers in the evaluation of assembler
>>> > directives. This generalizes and simplifies the extension from r334022
>>> > to catch another case found in the Linux kernel.
>>> >
>>> > Reviewers: rnk, void
>>> >
>>> > Reviewed By: rnk
>>> >
>>> > Subscribers: hiraditya, nickdesaulniers, llvm-commits
>>> >
>>> > Differential Revision: https://reviews.llvm.org/D50795
>>> >
>>> > Modified:
>>> >     llvm/trunk/include/llvm/MC/MCExpr.h
>>> >     llvm/trunk/include/llvm/MC/MCParser/MCAsmParserUtils.h
>>> >     llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h
>>> >     llvm/trunk/lib/MC/MCExpr.cpp
>>> >     llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>>> >     llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
>>> >     llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCExpr.h
>>> >     llvm/trunk/test/MC/X86/pr37425.s
>>> >
>>> > Modified: llvm/trunk/include/llvm/MC/MCExpr.h
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCExpr.h?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/include/llvm/MC/MCExpr.h (original)
>>> > +++ llvm/trunk/include/llvm/MC/MCExpr.h Thu Aug 16 09:31:14 2018
>>> > @@ -589,6 +589,8 @@ public:
>>> >    virtual bool evaluateAsRelocatableImpl(MCValue &Res,
>>> >                                           const MCAsmLayout *Layout,
>>> >                                           const MCFixup *Fixup) const =
>>> > 0;
>>> > +  // allow Target Expressions to be checked for equality
>>> > +  virtual bool isEqualTo(const MCExpr *x) const { return false; }
>>> >    // This should be set when assigned expressions are not valid ".set"
>>> >    // expressions, e.g. registers, and must be inlined.
>>> >    virtual bool inlineAssignedExpr() const { return false; }
>>> >
>>> > Modified: llvm/trunk/include/llvm/MC/MCParser/MCAsmParserUtils.h
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCParser/MCAsmParserUtils.h?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/include/llvm/MC/MCParser/MCAsmParserUtils.h (original)
>>> > +++ llvm/trunk/include/llvm/MC/MCParser/MCAsmParserUtils.h Thu Aug 16
>>> > 09:31:14 2018
>>> > @@ -25,7 +25,7 @@ namespace MCParserUtils {
>>> >  /// On success, returns false and sets the Symbol and Value output
>>> > parameters.
>>> >  bool parseAssignmentExpression(StringRef Name, bool allow_redef,
>>> >                                 MCAsmParser &Parser, MCSymbol *&Symbol,
>>> > -                               const MCExpr *&Value, bool
>>> > AllowExtendedExpr = false);
>>> > +                               const MCExpr *&Value);
>>> >
>>> >  } // namespace MCParserUtils
>>> >
>>> >
>>> > Modified: llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h (original)
>>> > +++ llvm/trunk/include/llvm/MC/MCParser/MCTargetAsmParser.h Thu Aug 16
>>> > 09:31:14 2018
>>> > @@ -372,9 +372,9 @@ public:
>>> >      SemaCallback = Callback;
>>> >    }
>>> >
>>> > -  // Target-specific parsing of assembler-level variable assignment.
>>> > -  virtual bool parseAssignmentExpression(const MCExpr *&Res, SMLoc
>>> > &EndLoc) {
>>> > -    return getParser().parseExpression(Res, EndLoc);
>>> > +  // Target-specific parsing of expression.
>>> > +  virtual bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
>>> > +    return getParser().parsePrimaryExpr(Res, EndLoc);
>>> >    }
>>> >
>>> >    virtual bool ParseRegister(unsigned &RegNo, SMLoc &StartLoc,
>>> >
>>> > Modified: llvm/trunk/lib/MC/MCExpr.cpp
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCExpr.cpp?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/lib/MC/MCExpr.cpp (original)
>>> > +++ llvm/trunk/lib/MC/MCExpr.cpp Thu Aug 16 09:31:14 2018
>>> > @@ -751,8 +751,22 @@ bool MCExpr::evaluateAsRelocatableImpl(M
>>> >      if (!ABE->getLHS()->evaluateAsRelocatableImpl(LHSValue, Asm,
>>> > Layout, Fixup,
>>> >                                                    Addrs, InSet) ||
>>> >          !ABE->getRHS()->evaluateAsRelocatableImpl(RHSValue, Asm,
>>> > Layout, Fixup,
>>> > -                                                  Addrs, InSet))
>>> > +                                                  Addrs, InSet)) {
>>> > +      // Check if both are Target Expressions, see if we can compare
>>> > them.
>>> > +      if (const MCTargetExpr *L =
>>> > dyn_cast<MCTargetExpr>(ABE->getLHS()))
>>> > +        if (const MCTargetExpr *R = cast<MCTargetExpr>(ABE->getRHS()))
>>> > {
>>> > +          switch (ABE->getOpcode()) {
>>> > +          case MCBinaryExpr::EQ:
>>> > +            Res = MCValue::get((L->isEqualTo(R)) ? -1 : 0);
>>> > +            return true;
>>> > +          case MCBinaryExpr::NE:
>>> > +            Res = MCValue::get((R->isEqualTo(R)) ? 0 : -1);
>>> > +            return true;
>>> > +          default: {}
>>> > +          }
>>> > +        }
>>> >        return false;
>>> > +    }
>>> >
>>> >      // We only support a few operations on non-constant expressions,
>>> > handle
>>> >      // those first.
>>> >
>>> > Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
>>> > +++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Thu Aug 16 09:31:14 2018
>>> > @@ -337,7 +337,7 @@ private:
>>> >    StringRef parseStringToComma();
>>> >
>>> >    bool parseAssignment(StringRef Name, bool allow_redef,
>>> > -                       bool NoDeadStrip = false, bool
>>> > AllowExtendedExpr = false);
>>> > +                       bool NoDeadStrip = false);
>>> >
>>> >    unsigned getBinOpPrecedence(AsmToken::TokenKind K,
>>> >                                MCBinaryExpr::Opcode &Kind);
>>> > @@ -1363,7 +1363,8 @@ void AsmParser::altMacroString(StringRef
>>> >  bool AsmParser::parseExpression(const MCExpr *&Res, SMLoc &EndLoc) {
>>> >    // Parse the expression.
>>> >    Res = nullptr;
>>> > -  if (parsePrimaryExpr(Res, EndLoc) || parseBinOpRHS(1, Res, EndLoc))
>>> > +  if (getTargetParser().parsePrimaryExpr(Res, EndLoc) ||
>>> > +      parseBinOpRHS(1, Res, EndLoc))
>>> >      return true;
>>> >
>>> >    // As a special case, we support 'a op b @ modifier' by rewriting
>>> > the
>>> > @@ -1617,7 +1618,7 @@ bool AsmParser::parseBinOpRHS(unsigned P
>>> >
>>> >      // Eat the next primary expression.
>>> >      const MCExpr *RHS;
>>> > -    if (parsePrimaryExpr(RHS, EndLoc))
>>> > +    if (getTargetParser().parsePrimaryExpr(RHS, EndLoc))
>>> >        return true;
>>> >
>>> >      // If BinOp binds less tightly with RHS than the operator after
>>> > RHS, let
>>> > @@ -1826,7 +1827,7 @@ bool AsmParser::parseStatement(ParseStat
>>> >      // identifier '=' ... -> assignment statement
>>> >      Lex();
>>> >
>>> > -    return parseAssignment(IDVal, true, /*NoDeadStrip*/ false,
>>> > /*AllowExtendedExpr*/true);
>>> > +    return parseAssignment(IDVal, true);
>>> >
>>> >    default: // Normal instruction or directive.
>>> >      break;
>>> > @@ -2766,11 +2767,11 @@ void AsmParser::handleMacroExit() {
>>> >  }
>>> >
>>> >  bool AsmParser::parseAssignment(StringRef Name, bool allow_redef,
>>> > -                                bool NoDeadStrip, bool
>>> > AllowExtendedExpr) {
>>> > +                                bool NoDeadStrip) {
>>> >    MCSymbol *Sym;
>>> >    const MCExpr *Value;
>>> >    if (MCParserUtils::parseAssignmentExpression(Name, allow_redef,
>>> > *this, Sym,
>>> > -                                               Value,
>>> > AllowExtendedExpr))
>>> > +                                               Value))
>>> >      return true;
>>> >
>>> >    if (!Sym) {
>>> > @@ -5839,17 +5840,13 @@ static bool isSymbolUsedInExpression(con
>>> >
>>> >  bool parseAssignmentExpression(StringRef Name, bool allow_redef,
>>> >                                 MCAsmParser &Parser, MCSymbol *&Sym,
>>> > -                               const MCExpr *&Value, bool
>>> > AllowExtendedExpr) {
>>> > +                               const MCExpr *&Value) {
>>> >
>>> >    // FIXME: Use better location, we should use proper tokens.
>>> >    SMLoc EqualLoc = Parser.getTok().getLoc();
>>> >    SMLoc EndLoc;
>>> > -  if (AllowExtendedExpr) {
>>> > -    if (Parser.getTargetParser().parseAssignmentExpression(Value,
>>> > EndLoc)) {
>>> > -      return Parser.TokError("missing expression");
>>> > -    }
>>> > -  } else if (Parser.parseExpression(Value, EndLoc))
>>> > -      return Parser.TokError("missing expression");
>>> > +  if (Parser.parseExpression(Value))
>>> > +    return Parser.TokError("missing expression");
>>> >
>>> >    // Note: we don't count b as used in "a = b". This is to allow
>>> >    // a = b
>>> >
>>> > 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=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
>>> > +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Thu Aug 16
>>> > 09:31:14 2018
>>> > @@ -955,7 +955,7 @@ public:
>>> >
>>> >    void SetFrameRegister(unsigned RegNo) override;
>>> >
>>> > -  bool parseAssignmentExpression(const MCExpr *&Res, SMLoc &EndLoc)
>>> > override;
>>> > +  bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) override;
>>> >
>>> >    bool ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
>>> >                          SMLoc NameLoc, OperandVector &Operands)
>>> > override;
>>> > @@ -2260,15 +2260,17 @@ std::unique_ptr<X86Operand> X86AsmParser
>>> >    return X86Operand::CreateMem(getPointerWidth(), Disp, MemStart,
>>> > MemEnd);
>>> >  }
>>> >
>>> > -// Parse either a standard expression or a register.
>>> > -bool X86AsmParser::parseAssignmentExpression(const MCExpr *&Res,
>>> > -                                             SMLoc &EndLoc) {
>>> > +// Parse either a standard primary expression or a register.
>>> > +bool X86AsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc)
>>> > {
>>> >    MCAsmParser &Parser = getParser();
>>> > -  if (Parser.parseExpression(Res, EndLoc)) {
>>> > +  if (Parser.parsePrimaryExpr(Res, EndLoc)) {
>>> >      SMLoc StartLoc = Parser.getTok().getLoc();
>>> >      // Normal Expression parse fails, check if it could be a register.
>>> >      unsigned RegNo;
>>> > -    if (Parser.getTargetParser().ParseRegister(RegNo, StartLoc,
>>> > EndLoc))
>>> > +    bool TryRegParse =
>>> > +        getTok().is(AsmToken::Percent) ||
>>> > +        (isParsingIntelSyntax() && getTok().is(AsmToken::Identifier));
>>> > +    if (!TryRegParse || ParseRegister(RegNo, StartLoc, EndLoc))
>>> >        return true;
>>> >      // Clear previous parse error and return correct expression.
>>> >      Parser.clearPendingErrors();
>>> >
>>> > Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCExpr.h
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCExpr.h?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCExpr.h (original)
>>> > +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCExpr.h Thu Aug 16
>>> > 09:31:14 2018
>>> > @@ -48,7 +48,7 @@ public:
>>> >    /// @}
>>> >
>>> >    void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override
>>> > {
>>> > -    if (MAI->getAssemblerDialect() == 0)
>>> > +    if (!MAI || MAI->getAssemblerDialect() == 0)
>>> >        OS << '%';
>>> >      OS << X86ATTInstPrinter::getRegisterName(RegNo);
>>> >    }
>>> > @@ -59,6 +59,11 @@ public:
>>> >    }
>>> >    // Register values should be inlined as they are not valid .set
>>> > expressions.
>>> >    bool inlineAssignedExpr() const override { return true; }
>>> > +  bool isEqualTo(const MCExpr *X) const override {
>>> > +    if (auto *E = dyn_cast<X86MCExpr>(X))
>>> > +      return getRegNo() == E->getRegNo();
>>> > +    return false;
>>> > +  }
>>> >    void visitUsedExpr(MCStreamer &Streamer) const override{};
>>> >    MCFragment *findAssociatedFragment() const override { return
>>> > nullptr; }
>>> >
>>> >
>>> > Modified: llvm/trunk/test/MC/X86/pr37425.s
>>> > URL:
>>> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/pr37425.s?rev=339895&r1=339894&r2=339895&view=diff
>>> >
>>> > ==============================================================================
>>> > --- llvm/trunk/test/MC/X86/pr37425.s (original)
>>> > +++ llvm/trunk/test/MC/X86/pr37425.s Thu Aug 16 09:31:14 2018
>>> > @@ -1,5 +1,4 @@
>>> > -// RUN: llvm-mc -triple x86_64-unknown-unknown -defsym=ERR=0 %s -o -
>>> > | FileCheck %s
>>> > -// RUN: not llvm-mc -triple x86_64-unknown-unknown -defsym=ERR=1 %s -o
>>> > - 2>&1 | FileCheck --check-prefix=ERR %s
>>> > +// RUN: llvm-mc -triple x86_64-unknown-unknown %s -o -      |
>>> > FileCheck %s
>>> >
>>> >  // CHECK-NOT: .set var_xdata
>>> >  var_xdata = %rcx
>>> > @@ -7,10 +6,15 @@ var_xdata = %rcx
>>> >  // CHECK: xorq %rcx, %rcx
>>> >  xorq var_xdata, var_xdata
>>> >
>>> > -.if (ERR==1)
>>> > -// ERR: [[@LINE+2]]:15: error: unknown token in expression in '.set'
>>> > directive
>>> > -// ERR: [[@LINE+1]]:15: error: missing expression in '.set' directive
>>> > -.set err_var, %rcx
>>> > -.endif
>>> > +// CHECK: .data
>>> > +// CHECK-NEXT: .byte 1
>>> > +.data
>>> > +.if var_xdata == %rax
>>> > +  .byte 0
>>> > +.elseif var_xdata == %rcx
>>> > +  .byte 1
>>> > +.else
>>> > +  .byte 2
>>> > +.endif
>>> >
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at lists.llvm.org
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>


More information about the llvm-commits mailing list