From 1bee6340efca945609438fcac0d6c149952d97d0 Mon Sep 17 00:00:00 2001 From: Michael Zuckerman Date: Tue, 18 Oct 2016 13:52:39 +0000 Subject: [PATCH] [x86][inline-asm][avx512] allow swapping of '{k}' & '{z}' marks Committing on behalf of Coby Tayree: After check-all and LGTM Desc: AVX512 allows dest operand to be followed by an op-mask register specifier ('{k}', which in turn may be followed by a merging/zeroing specifier ('{z}') Currently, the following forms are allowed: {k} {k}{z} This patch allows the following forms: {z}{k} and ignores the next form: {z} Justification would be quite simple - GCC Differential Revision: http://reviews.llvm.org/D25013 llvm-svn: 284479 --- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp | 90 +++++++++++++++++++------- llvm/test/MC/X86/avx512bw-encoding.s | 4 ++ llvm/test/MC/X86/intel-syntax-avx512.s | 4 ++ 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp index d51cd59..f680728 100644 --- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp +++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp @@ -759,10 +759,12 @@ private: /// Parses AVX512 specific operand primitives: masked registers ({%k}, {z}) /// and memory broadcasting ({1to}) primitives, updating Operands vector if required. - /// \return \c true if no parsing errors occurred, \c false otherwise. + /// return false if no parsing errors occurred, true otherwise. bool HandleAVX512Operand(OperandVector &Operands, const MCParsedAsmOperand &Op); + bool ParseZ(std::unique_ptr &Z, const SMLoc &StartLoc); + bool is64BitMode() const { // FIXME: Can tablegen auto-generate this? return getSTI().getFeatureBits()[X86::Mode64Bit]; @@ -1907,6 +1909,28 @@ std::unique_ptr X86AsmParser::ParseATTOperand() { } } +// true on failure, false otherwise +// If no {z} mark was found - Parser doesn't advance +bool X86AsmParser::ParseZ(std::unique_ptr &Z, + const SMLoc &StartLoc) { + MCAsmParser &Parser = getParser(); + // Assuming we are just pass the '{' mark, quering the next token + // Searched for {z}, but none was found. Return true, as no parsing error was + // encountered + if (!(getLexer().is(AsmToken::Identifier) && + (getLexer().getTok().getIdentifier() == "z"))) + return false; + Parser.Lex(); // Eat z + // Query and eat the '}' mark + if (!getLexer().is(AsmToken::RCurly)) + return Error(getLexer().getLoc(), "Expected } at this point"); + Parser.Lex(); // Eat '}' + // Assign Z with the {z} mark opernad + Z.reset(X86Operand::CreateToken("{z}", StartLoc).release()); + return false; +} + +// true on failure, false otherwise bool X86AsmParser::HandleAVX512Operand(OperandVector &Operands, const MCParsedAsmOperand &Op) { MCAsmParser &Parser = getParser(); @@ -1918,11 +1942,11 @@ bool X86AsmParser::HandleAVX512Operand(OperandVector &Operands, if(getLexer().is(AsmToken::Integer)) { // Parse memory broadcasting ({1to}). if (getLexer().getTok().getIntVal() != 1) - return !TokError("Expected 1to at this point"); + return TokError("Expected 1to at this point"); Parser.Lex(); // Eat "1" of 1to8 if (!getLexer().is(AsmToken::Identifier) || !getLexer().getTok().getIdentifier().startswith("to")) - return !TokError("Expected 1to at this point"); + return TokError("Expected 1to at this point"); // Recognize only reasonable suffixes. const char *BroadcastPrimitive = StringSwitch(getLexer().getTok().getIdentifier()) @@ -1932,41 +1956,57 @@ bool X86AsmParser::HandleAVX512Operand(OperandVector &Operands, .Case("to16", "{1to16}") .Default(nullptr); if (!BroadcastPrimitive) - return !TokError("Invalid memory broadcast primitive."); + return TokError("Invalid memory broadcast primitive."); Parser.Lex(); // Eat "toN" of 1toN if (!getLexer().is(AsmToken::RCurly)) - return !TokError("Expected } at this point"); + return TokError("Expected } at this point"); Parser.Lex(); // Eat "}" Operands.push_back(X86Operand::CreateToken(BroadcastPrimitive, consumedToken)); // No AVX512 specific primitives can pass // after memory broadcasting, so return. - return true; + return false; } else { - // Parse mask register {%k1} - Operands.push_back(X86Operand::CreateToken("{", consumedToken)); - if (std::unique_ptr Op = ParseOperand()) { - Operands.push_back(std::move(Op)); - if (!getLexer().is(AsmToken::RCurly)) - return !TokError("Expected } at this point"); - Operands.push_back(X86Operand::CreateToken("}", consumeToken())); - - // Parse "zeroing non-masked" semantic {z} - if (getLexer().is(AsmToken::LCurly)) { - Operands.push_back(X86Operand::CreateToken("{z}", consumeToken())); - if (!getLexer().is(AsmToken::Identifier) || - getLexer().getTok().getIdentifier() != "z") - return !TokError("Expected z at this point"); - Parser.Lex(); // Eat the z + // Parse either {k}{z}, {z}{k}, {k} or {z} + // last one have no meaning, but GCC accepts it + // Currently, we're just pass a '{' mark + std::unique_ptr Z; + if (ParseZ(Z, consumedToken)) + return true; + // Reaching here means that parsing of the allegadly '{z}' mark yielded + // no errors. + // Query for the need of further parsing for a {%k} mark + if (!Z || getLexer().is(AsmToken::LCurly)) { + const SMLoc StartLoc = Z ? consumeToken() : consumedToken; + // Parse an op-mask register mark ({%k}), which is now to be + // expected + if (std::unique_ptr Op = ParseOperand()) { if (!getLexer().is(AsmToken::RCurly)) - return !TokError("Expected } at this point"); - Parser.Lex(); // Eat the } + return Error(getLexer().getLoc(), "Expected } at this point"); + Operands.push_back(X86Operand::CreateToken("{", StartLoc)); + Operands.push_back(std::move(Op)); + Operands.push_back(X86Operand::CreateToken("}", consumeToken())); + } else + return Error(getLexer().getLoc(), + "Expected an op-mask register at this point"); + // {%k} mark is found, inquire for {z} + if (getLexer().is(AsmToken::LCurly) && !Z) { + // Have we've found a parsing error, or found no (expected) {z} mark + // - report an error + if (ParseZ(Z, consumeToken()) || !Z) + return true; + } + // '{z}' on its own is meaningless, hence should be ignored. + // on the contrary - have it been accompanied by a K register, + // allow it. + if (Z) + Operands.push_back(std::move(Z)); } } } } - return true; + return false; } /// ParseMemOperand: segment: disp(basereg, indexreg, scale). The '%ds:' prefix @@ -2323,7 +2363,7 @@ bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name, while(1) { if (std::unique_ptr Op = ParseOperand()) { Operands.push_back(std::move(Op)); - if (!HandleAVX512Operand(Operands, *Operands.back())) + if (HandleAVX512Operand(Operands, *Operands.back())) return true; } else { return true; diff --git a/llvm/test/MC/X86/avx512bw-encoding.s b/llvm/test/MC/X86/avx512bw-encoding.s index 954e0bd..aaf001a 100644 --- a/llvm/test/MC/X86/avx512bw-encoding.s +++ b/llvm/test/MC/X86/avx512bw-encoding.s @@ -12,6 +12,10 @@ // CHECK: encoding: [0x62,0x82,0x6d,0xc5,0x66,0xc9] vpblendmb %zmm25, %zmm18, %zmm17 {%k5} {z} +// CHECK: vpblendmb %zmm25, %zmm18, %zmm17 {%k5} {z} +// CHECK: encoding: [0x62,0x82,0x6d,0xc5,0x66,0xc9] + vpblendmb %zmm25, %zmm18, %zmm17 {z} {%k5} + // CHECK: vpblendmb (%rcx), %zmm18, %zmm17 // CHECK: encoding: [0x62,0xe2,0x6d,0x40,0x66,0x09] vpblendmb (%rcx), %zmm18, %zmm17 diff --git a/llvm/test/MC/X86/intel-syntax-avx512.s b/llvm/test/MC/X86/intel-syntax-avx512.s index 2705e8c..16351d1 100644 --- a/llvm/test/MC/X86/intel-syntax-avx512.s +++ b/llvm/test/MC/X86/intel-syntax-avx512.s @@ -16,6 +16,10 @@ vaddpd zmm1 {k5}, zmm1, zmm2 // CHECK: encoding: [0x62,0xf1,0xf5,0xcd,0x58,0xca] vaddpd zmm1 {k5} {z}, zmm1, zmm2 +// CHECK: vaddpd zmm1 {k5} {z}, zmm1, zmm2 +// CHECK: encoding: [0x62,0xf1,0xf5,0xcd,0x58,0xca] +vaddpd zmm1 {z} {k5}, zmm1, zmm2 + // CHECK: vaddpd zmm1, zmm1, zmm2, {rn-sae} // CHECK: encoding: [0x62,0xf1,0xf5,0x18,0x58,0xca] vaddpd zmm1, zmm1, zmm2, {rn-sae} -- 2.7.4