llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-aarch64 Author: Jonathan Thackray (jthackray) <details> <summary>Changes</summary> The code which handles instructions that are aliased in the `HINT` encoding space is very similar and repetitive. Move common code into templated functions, so that it's consistent and simpler, whilst still remaining readable. This also means any future instructions added in the `HINT` space will be simpler to implement. Net removal of ~86 lines of code. NFC, apart from a word change in the `tsb csync` error diagnostic --- Patch is 21.80 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/196318.diff 4 Files Affected: - (modified) llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp (+99-195) - (modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp (+44-43) - (modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.h (+9) - (modified) llvm/test/MC/AArch64/armv8.4a-trace-error.s (+3-3) ``````````diff diff --git a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp index eeb0219998252..b992e97b75feb 100644 --- a/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp +++ b/llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp @@ -302,6 +302,24 @@ class AArch64AsmParser : public MCTargetAsmParser { ParseStatus tryParseImmRange(OperandVector &Operands); template <int> ParseStatus tryParseAdjImm0_63(OperandVector &Operands); ParseStatus tryParsePHintInstOperand(OperandVector &Operands); + template <typename LookupFn, typename CreateFn> + ParseStatus tryParseNamedHintOperand(OperandVector &Operands, + LookupFn LookupByName, + CreateFn CreateOperand) { + SMLoc S = getLoc(); + const AsmToken &Tok = getTok(); + if (Tok.isNot(AsmToken::Identifier)) + return TokError("invalid operand for instruction"); + + auto Entry = LookupByName(Tok.getString()); + if (!Entry) + return TokError("invalid operand for instruction"); + + Operands.push_back( + CreateOperand(Entry->Encoding, Tok.getString(), S, getContext())); + Lex(); // Eat identifier token. + return ParseStatus::Success; + } public: enum AArch64MatchResultTy { @@ -494,41 +512,18 @@ class AArch64Operand : public MCParsedAsmOperand { unsigned Val; }; - struct PSBHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct PHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct BTIHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct SHUHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct TSBHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct CMHPriorityHintOp { - const char *Data; - unsigned Length; - unsigned Val; - }; - struct TIndexHintOp { + struct NamedHintOp { const char *Data; unsigned Length; unsigned Val; }; + using PSBHintOp = NamedHintOp; + using PHintOp = NamedHintOp; + using BTIHintOp = NamedHintOp; + using SHUHintOp = NamedHintOp; + using TSBHintOp = NamedHintOp; + using CMHPriorityHintOp = NamedHintOp; + using TIndexHintOp = NamedHintOp; struct SVCROp { const char *Data; @@ -537,30 +532,30 @@ class AArch64Operand : public MCParsedAsmOperand { }; union { - struct TokOp Tok; - struct RegOp Reg; - struct MatrixRegOp MatrixReg; - struct MatrixTileListOp MatrixTileList; - struct VectorListOp VectorList; - struct VectorIndexOp VectorIndex; - struct ImmOp Imm; - struct ShiftedImmOp ShiftedImm; - struct ImmRangeOp ImmRange; - struct CondCodeOp CondCode; - struct FPImmOp FPImm; - struct BarrierOp Barrier; - struct SysRegOp SysReg; - struct SysCRImmOp SysCRImm; - struct PrefetchOp Prefetch; - struct PSBHintOp PSBHint; - struct PHintOp PHint; - struct BTIHintOp BTIHint; - struct SHUHintOp SHUHint; - struct TSBHintOp TSBHint; - struct CMHPriorityHintOp CMHPriorityHint; - struct TIndexHintOp TIndexHint; - struct ShiftExtendOp ShiftExtend; - struct SVCROp SVCR; + TokOp Tok; + RegOp Reg; + MatrixRegOp MatrixReg; + MatrixTileListOp MatrixTileList; + VectorListOp VectorList; + VectorIndexOp VectorIndex; + ImmOp Imm; + ShiftedImmOp ShiftedImm; + ImmRangeOp ImmRange; + CondCodeOp CondCode; + FPImmOp FPImm; + BarrierOp Barrier; + SysRegOp SysReg; + SysCRImmOp SysCRImm; + PrefetchOp Prefetch; + PSBHintOp PSBHint; + PHintOp PHint; + BTIHintOp BTIHint; + SHUHintOp SHUHint; + TSBHintOp TSBHint; + CMHPriorityHintOp CMHPriorityHint; + TIndexHintOp TIndexHint; + ShiftExtendOp ShiftExtend; + SVCROp SVCR; }; // Keep the MCContext around as the MCExprs may need manipulated during @@ -2594,17 +2589,31 @@ class AArch64Operand : public MCParsedAsmOperand { return Op; } + struct IdentityHintEncoding { + unsigned operator()(unsigned Val) const { return Val; } + }; + + template <KindTy Kind, NamedHintOp AArch64Operand::*Member, + typename Encode = IdentityHintEncoding> static std::unique_ptr<AArch64Operand> - CreatePHintInst(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_PHint, Ctx); - Op->PHint.Val = Val; - Op->PHint.Data = Str.data(); - Op->PHint.Length = Str.size(); + CreateNamedHintOperand(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx, + Encode EncodeVal = {}) { + auto Op = std::make_unique<AArch64Operand>(Kind, Ctx); + auto &Hint = Op.get()->*Member; + Hint.Val = EncodeVal(Val); + Hint.Data = Str.data(); + Hint.Length = Str.size(); Op->StartLoc = S; Op->EndLoc = S; return Op; } + static std::unique_ptr<AArch64Operand> + CreatePHintInst(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { + return CreateNamedHintOperand<k_PHint, &AArch64Operand::PHint>(Val, Str, S, + Ctx); + } + static std::unique_ptr<AArch64Operand> CreateSysCR(unsigned Val, SMLoc S, SMLoc E, MCContext &Ctx) { auto Op = std::make_unique<AArch64Operand>(k_SysCR, Ctx); @@ -2631,70 +2640,41 @@ class AArch64Operand : public MCParsedAsmOperand { StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_PSBHint, Ctx); - Op->PSBHint.Val = Val; - Op->PSBHint.Data = Str.data(); - Op->PSBHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_PSBHint, &AArch64Operand::PSBHint>(Val, Str, + S, Ctx); } static std::unique_ptr<AArch64Operand> CreateBTIHint(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_BTIHint, Ctx); - Op->BTIHint.Val = Val | 32; - Op->BTIHint.Data = Str.data(); - Op->BTIHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_BTIHint, &AArch64Operand::BTIHint>( + Val, Str, S, Ctx, [](unsigned EncodedVal) { return EncodedVal | 32; }); } static std::unique_ptr<AArch64Operand> CreateSHUHint(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_SHUHint, Ctx); - Op->SHUHint.Val = Val + 50; - Op->SHUHint.Data = Str.data(); - Op->SHUHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_SHUHint, &AArch64Operand::SHUHint>( + Val, Str, S, Ctx, [](unsigned EncodedVal) { return EncodedVal + 50; }); } static std::unique_ptr<AArch64Operand> CreateTSBHint(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_TSBHint, Ctx); - Op->TSBHint.Val = Val | 16; - Op->TSBHint.Data = Str.data(); - Op->TSBHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_TSBHint, &AArch64Operand::TSBHint>( + Val, Str, S, Ctx, [](unsigned EncodedVal) { return EncodedVal | 16; }); } static std::unique_ptr<AArch64Operand> CreateCMHPriorityHint(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_CMHPriorityHint, Ctx); - Op->CMHPriorityHint.Val = Val; - Op->CMHPriorityHint.Data = Str.data(); - Op->CMHPriorityHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_CMHPriorityHint, + &AArch64Operand::CMHPriorityHint>(Val, Str, S, + Ctx); } static std::unique_ptr<AArch64Operand> CreateTIndexHint(unsigned Val, StringRef Str, SMLoc S, MCContext &Ctx) { - auto Op = std::make_unique<AArch64Operand>(k_TIndexHint, Ctx); - Op->TIndexHint.Val = Val; - Op->TIndexHint.Data = Str.data(); - Op->TIndexHint.Length = Str.size(); - Op->StartLoc = S; - Op->EndLoc = S; - return Op; + return CreateNamedHintOperand<k_TIndexHint, &AArch64Operand::TIndexHint>( + Val, Str, S, Ctx); } static std::unique_ptr<AArch64Operand> @@ -3377,19 +3357,8 @@ ParseStatus AArch64AsmParser::tryParsePrefetch(OperandVector &Operands) { /// tryParsePSBHint - Try to parse a PSB operand, mapped to Hint command ParseStatus AArch64AsmParser::tryParsePSBHint(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto PSB = AArch64PSBHint::lookupPSBByName(Tok.getString()); - if (!PSB) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreatePSBHint( - PSB->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand(Operands, AArch64PSBHint::lookupPSBByName, + AArch64Operand::CreatePSBHint); } ParseStatus AArch64AsmParser::tryParseSyspXzrPair(OperandVector &Operands) { @@ -3427,54 +3396,22 @@ ParseStatus AArch64AsmParser::tryParseSyspXzrPair(OperandVector &Operands) { /// tryParseBTIHint - Try to parse a BTI operand, mapped to Hint command ParseStatus AArch64AsmParser::tryParseBTIHint(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto BTI = AArch64BTIHint::lookupBTIByName(Tok.getString()); - if (!BTI) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreateBTIHint( - BTI->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand(Operands, AArch64BTIHint::lookupBTIByName, + AArch64Operand::CreateBTIHint); } /// tryParseCMHPriorityHint - Try to parse a CMHPriority operand ParseStatus AArch64AsmParser::tryParseCMHPriorityHint(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto CMHPriority = - AArch64CMHPriorityHint::lookupCMHPriorityHintByName(Tok.getString()); - if (!CMHPriority) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreateCMHPriorityHint( - CMHPriority->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand( + Operands, AArch64CMHPriorityHint::lookupCMHPriorityHintByName, + AArch64Operand::CreateCMHPriorityHint); } /// tryParseTIndexHint - Try to parse a TIndex operand ParseStatus AArch64AsmParser::tryParseTIndexHint(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto TIndex = AArch64TIndexHint::lookupTIndexByName(Tok.getString()); - if (!TIndex) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreateTIndexHint( - TIndex->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand(Operands, + AArch64TIndexHint::lookupTIndexByName, + AArch64Operand::CreateTIndexHint); } /// tryParseAdrpLabel - Parse and validate a source label for the ADRP @@ -4505,52 +4442,19 @@ ParseStatus AArch64AsmParser::tryParseSysReg(OperandVector &Operands) { ParseStatus AArch64AsmParser::tryParsePHintInstOperand(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto PH = AArch64PHint::lookupPHintByName(Tok.getString()); - if (!PH) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreatePHintInst( - PH->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand(Operands, AArch64PHint::lookupPHintByName, + AArch64Operand::CreatePHintInst); } ParseStatus AArch64AsmParser::tryParseSHUHintOperand(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("invalid operand for instruction"); - - auto SHU = - AArch64CMHPriorityHint::lookupCMHPriorityHintByName(Tok.getString()); - if (!SHU) - return TokError("invalid operand for instruction"); - - Operands.push_back(AArch64Operand::CreateSHUHint( - SHU->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand( + Operands, AArch64CMHPriorityHint::lookupCMHPriorityHintByName, + AArch64Operand::CreateSHUHint); } ParseStatus AArch64AsmParser::tryParseTSBHintOperand(OperandVector &Operands) { - SMLoc S = getLoc(); - const AsmToken &Tok = getTok(); - if (Tok.isNot(AsmToken::Identifier)) - return TokError("'csync' operand expected"); - - auto TSB = AArch64TSB::lookupTSBByName(Tok.getString()); - if (!TSB || TSB->Encoding != AArch64TSB::csync) - return TokError("'csync' operand expected"); - - Operands.push_back(AArch64Operand::CreateTSBHint( - TSB->Encoding, Tok.getString(), S, getContext())); - Lex(); // Eat identifier token. - return ParseStatus::Success; + return tryParseNamedHintOperand(Operands, AArch64TSB::lookupTSBByName, + AArch64Operand::CreateTSBHint); } /// tryParseNeonVectorRegister - Parse a vector register operand. diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp index 8ae2acca041a3..ed21673ac7588 100644 --- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp +++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp @@ -1571,72 +1571,77 @@ void AArch64InstPrinter::printPrefetchOp(const MCInst *MI, unsigned OpNum, markup(O, Markup::Immediate) << '#' << formatImm(prfop); } +unsigned AArch64InstPrinter::decodeIdentityHint(unsigned Value) { + return Value; +} + +unsigned AArch64InstPrinter::decodeBTIHint(unsigned Value) { + return Value ^ 32; +} + +unsigned AArch64InstPrinter::decodeSHUHint(unsigned Value) { + return Value - 50; +} + +unsigned AArch64InstPrinter::decodeTSBHint(unsigned Value) { + return Value ^ 16; +} + +template <typename LookupFn, typename DecodeFn> +void AArch64InstPrinter::printNamedHintOp(unsigned Encoded, raw_ostream &O, + LookupFn LookupByEncoding, + DecodeFn DecodeValue) { + unsigned Decoded = DecodeValue(Encoded); + if (auto Hint = LookupByEncoding(Decoded)) + O << Hint->Name; + else + markup(O, Markup::Immediate) << '#' << formatImm(Decoded); +} + void AArch64InstPrinter::printPSBHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned psbhintop = MI->getOperand(OpNum).getImm(); - auto PSB = AArch64PSBHint::lookupPSBByEncoding(psbhintop); - if (PSB) - O << PSB->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(psbhintop); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64PSBHint::lookupPSBByEncoding, decodeIdentityHint); } void AArch64InstPrinter::printBTIHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned btihintop = MI->getOperand(OpNum).getImm() ^ 32; - auto BTI = AArch64BTIHint::lookupBTIByEncoding(btihintop); - if (BTI) - O << BTI->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(btihintop); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64BTIHint::lookupBTIByEncoding, decodeBTIHint); } void AArch64InstPrinter::printSHUHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned shuhintop = MI->getOperand(OpNum).getImm() - 50; - auto SHU = AArch64CMHPriorityHint::lookupCMHPriorityHintByEncoding(shuhintop); - if (SHU) - O << SHU->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(shuhintop); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64CMHPriorityHint::lookupCMHPriorityHintByEncoding, + decodeSHUHint); } void AArch64InstPrinter::printTSBHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned tsbhintop = MI->getOperand(OpNum).getImm() ^ 16; - auto TSB = AArch64TSB::lookupTSBByEncoding(tsbhintop); - if (TSB) - O << TSB->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(tsbhintop); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64TSB::lookupTSBByEncoding, decodeTSBHint); } void AArch64InstPrinter::printCMHPriorityHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned priorityhint_op = MI->getOperand(OpNum).getImm(); - auto PHint = - AArch64CMHPriorityHint::lookupCMHPriorityHintByEncoding(priorityhint_op); - if (PHint) - O << PHint->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(priorityhint_op); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64CMHPriorityHint::lookupCMHPriorityHintByEncoding, + decodeIdentityHint); } void AArch64InstPrinter::printTIndexHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned tindexhintop = MI->getOperand(OpNum).getImm(); - auto TIndex = AArch64TIndexHint::lookupTIndexByEncoding(tindexhintop); - if (TIndex) - O << TIndex->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(tindexhintop); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64TIndexHint::lookupTIndexByEncoding, + decodeIdentityHint); } void AArch64InstPrinter::printFPImmOperand(const MCInst *MI, unsigned OpNum, @@ -2283,10 +2288,6 @@ void AArch64InstPrinter::printSyspXzrPair(const MCInst *MI, unsigned OpNum, void AArch64InstPrinter::printPHintOp(const MCInst *MI, unsigned OpNum, const MCSubtargetInfo &STI, raw_ostream &O) { - unsigned Op = MI->getOperand(OpNum).getImm(); - auto PH = AArch64PHint::lookupPHintByEncoding(Op); - if (PH) - O << PH->Name; - else - markup(O, Markup::Immediate) << '#' << formatImm(Op); + printNamedHintOp(MI->getOperand(OpNum).getImm(), O, + AArch64PHint::lookupPHintByEncoding, decodeIdentityHint); } diff --git a/llvm/lib/Target/AArch64/MCTar... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/196318 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
