[llvm] r339895 - [MC][X86] Enhance X86 Register expression handling to more closely match GCC.
Nirav Davé via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 21 10:59:25 PDT 2018
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
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180821/9cff932e/attachment.html>
More information about the llvm-commits
mailing list