[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 09:58:28 PDT 2018


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