From: Dmitry Preobrazhensky Date: Mon, 22 Apr 2019 14:35:47 +0000 (+0000) Subject: [AMDGPU][MC] Corrected parsing of SP3 'neg' modifier X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=31bd08af2d5ea374639b5ebf7ce4ba79d721bc56;p=llvm [AMDGPU][MC] Corrected parsing of SP3 'neg' modifier See bug 41156: https://bugs.llvm.org/show_bug.cgi?id=41156 Reviewers: artem.tamazov, arsenm Differential Revision: https://reviews.llvm.org/D60624 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358888 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp index 397f483f726..7fcfc0b2901 100644 --- a/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp +++ b/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp @@ -1080,6 +1080,7 @@ public: StringRef &Value); bool parseAbsoluteExpr(int64_t &Val, bool HasSP3AbsModifier = false); + bool parseSP3NegModifier(); OperandMatchResultTy parseImm(OperandVector &Operands, bool HasSP3AbsModifier = false); OperandMatchResultTy parseReg(OperandVector &Operands); OperandMatchResultTy parseRegOrImm(OperandVector &Operands, bool AbsMod = false); @@ -1134,6 +1135,7 @@ private: bool trySkipToken(const AsmToken::TokenKind Kind); bool skipToken(const AsmToken::TokenKind Kind, const StringRef ErrMsg); bool parseString(StringRef &Val, const StringRef ErrMsg = "expected a string"); + void peekTokens(MutableArrayRef Tokens); AsmToken::TokenKind getTokenKind() const; bool parseExpr(int64_t &Imm); StringRef getTokenStr() const; @@ -2104,34 +2106,58 @@ AMDGPUAsmParser::parseRegOrImm(OperandVector &Operands, bool AbsMod) { res; } +// Check if the current token is an SP3 'neg' modifier. +// Currently this modifier is allowed in the following context: +// +// 1. Before a register, e.g. "-v0", "-v[...]" or "-[v0,v1]". +// 2. Before an 'abs' modifier: -abs(...) +// 3. Before an SP3 'abs' modifier: -|...| +// +// In all other cases "-" is handled as a part +// of an expression that follows the sign. +// +// Note: When "-" is followed by an integer literal, +// this is interpreted as integer negation rather +// than a floating-point NEG modifier applied to N. +// Beside being contr-intuitive, such use of floating-point +// NEG modifier would have resulted in different meaning +// of integer literals used with VOP1/2/C and VOP3, +// for example: +// v_exp_f32_e32 v5, -1 // VOP1: src0 = 0xFFFFFFFF +// v_exp_f32_e64 v5, -1 // VOP3: src0 = 0x80000001 +// Negative fp literals with preceding "-" are +// handled likewise for unifomtity +// +bool +AMDGPUAsmParser::parseSP3NegModifier() { + + AsmToken NextToken[2]; + peekTokens(NextToken); + + if (isToken(AsmToken::Minus) && + (isRegister(NextToken[0], NextToken[1]) || + NextToken[0].is(AsmToken::Pipe) || + isId(NextToken[0], "abs"))) { + lex(); + return true; + } + + return false; +} + OperandMatchResultTy AMDGPUAsmParser::parseRegOrImmWithFPInputMods(OperandVector &Operands, bool AllowImm) { - bool Negate = false, Negate2 = false, Abs = false, Abs2 = false; + bool Negate, Negate2 = false, Abs = false, Abs2 = false; - if (getLexer().getKind()== AsmToken::Minus) { - const AsmToken NextToken = getLexer().peekTok(); - - // Disable ambiguous constructs like '--1' etc. Should use neg(-1) instead. - if (NextToken.is(AsmToken::Minus)) { - Error(Parser.getTok().getLoc(), "invalid syntax, expected 'neg' modifier"); - return MatchOperand_ParseFail; - } - - // '-' followed by an integer literal N should be interpreted as integer - // negation rather than a floating-point NEG modifier applied to N. - // Beside being contr-intuitive, such use of floating-point NEG modifier - // results in different meaning of integer literals used with VOP1/2/C - // and VOP3, for example: - // v_exp_f32_e32 v5, -1 // VOP1: src0 = 0xFFFFFFFF - // v_exp_f32_e64 v5, -1 // VOP3: src0 = 0x80000001 - // Negative fp literals should be handled likewise for unifomtity - if (!NextToken.is(AsmToken::Integer) && !NextToken.is(AsmToken::Real)) { - Parser.Lex(); - Negate = true; - } + // Disable ambiguous constructs like '--1' etc. Should use neg(-1) instead. + if (isToken(AsmToken::Minus) && peekToken().is(AsmToken::Minus)) { + Error(getLoc(), "invalid syntax, expected 'neg' modifier"); + return MatchOperand_ParseFail; } + Negate = parseSP3NegModifier(); + if (getLexer().getKind() == AsmToken::Identifier && Parser.getTok().getString() == "neg") { if (Negate) { @@ -2174,7 +2200,7 @@ AMDGPUAsmParser::parseRegOrImmWithFPInputMods(OperandVector &Operands, Res = parseReg(Operands); } if (Res != MatchOperand_Success) { - return Res; + return (Negate || Negate2 || Abs || Abs2)? MatchOperand_ParseFail : Res; } AMDGPUOperand::Modifiers Mods; @@ -2236,7 +2262,7 @@ AMDGPUAsmParser::parseRegOrImmWithIntInputMods(OperandVector &Operands, Res = parseReg(Operands); } if (Res != MatchOperand_Success) { - return Res; + return Sext? MatchOperand_ParseFail : Res; } AMDGPUOperand::Modifiers Mods; @@ -4637,6 +4663,14 @@ AMDGPUAsmParser::peekToken() { return getLexer().peekTok(); } +void +AMDGPUAsmParser::peekTokens(MutableArrayRef Tokens) { + auto TokCount = getLexer().peekTokens(Tokens); + + for (auto Idx = TokCount; Idx < Tokens.size(); ++Idx) + Tokens[Idx] = AsmToken(AsmToken::Error, ""); +} + AsmToken::TokenKind AMDGPUAsmParser::getTokenKind() const { return getLexer().getKind(); diff --git a/test/MC/AMDGPU/expressions.s b/test/MC/AMDGPU/expressions.s index 72f8ae4ce11..01dd3bb1269 100644 --- a/test/MC/AMDGPU/expressions.s +++ b/test/MC/AMDGPU/expressions.s @@ -72,3 +72,31 @@ s_sub_u32 s0, s0, -1.0 + 10000000000 t=10000000000 s_sub_u32 s0, s0, 1.0 + t // NOVI: error: invalid operand for instruction + +//===----------------------------------------------------------------------===// +// Symbols may look like registers. +// They should be allowed in expressions if there is no ambiguity. +//===----------------------------------------------------------------------===// + +v=1 +v_sin_f32 v0, -v +// VI: v_sin_f32_e32 v0, -1 ; encoding: [0xc1,0x52,0x00,0x7e] + +v_sin_f32 v0, -v[0] +// VI: v_sin_f32_e64 v0, -v0 ; encoding: [0x00,0x00,0x69,0xd1,0x00,0x01,0x00,0x20] + +s=1 +v_sin_f32 v0, -s +// VI: v_sin_f32_e32 v0, -1 ; encoding: [0xc1,0x52,0x00,0x7e] + +s0=1 +v_sin_f32 v0, -s0 +// VI: v_sin_f32_e64 v0, -s0 ; encoding: [0x00,0x00,0x69,0xd1,0x00,0x00,0x00,0x20] + +ttmp=1 +v_sin_f32 v0, -ttmp +// VI: v_sin_f32_e32 v0, -1 ; encoding: [0xc1,0x52,0x00,0x7e] + +ttmp0=1 +v_sin_f32 v0, -[ttmp0] +// VI: v_sin_f32_e64 v0, -ttmp0 ; encoding: [0x00,0x00,0x69,0xd1,0x70,0x00,0x00,0x20] diff --git a/test/MC/AMDGPU/vop3-modifiers-err.s b/test/MC/AMDGPU/vop3-modifiers-err.s index bd08ee2d10a..b28768c1ca0 100644 --- a/test/MC/AMDGPU/vop3-modifiers-err.s +++ b/test/MC/AMDGPU/vop3-modifiers-err.s @@ -12,4 +12,4 @@ v_ceil_f32 v0, --1 // CHECK: error: invalid syntax, expected 'neg' modifier v_ceil_f16 v0, abs(neg(1)) -// CHECK: error: not a valid operand \ No newline at end of file +// CHECK: error: failed parsing operand