From e8e92c8313a0481ad2770587310d399abddce665 Mon Sep 17 00:00:00 2001 From: Michael Maitland Date: Tue, 15 Nov 2022 18:01:19 -0800 Subject: [PATCH] Revert "[RISCV][llvm-mca] Use LMUL Instruments to provide more accurate reports on RISCV" This reverts commit b88b8307bf9e24f53e7ef3052abf2c506ff55fd2. --- llvm/docs/CommandGuide/llvm-mca.rst | 123 +---------------- llvm/include/llvm/MC/TargetRegistry.h | 38 ------ llvm/include/llvm/MCA/CustomBehaviour.h | 56 -------- llvm/include/llvm/MCA/InstrBuilder.h | 28 +--- llvm/lib/MCA/CustomBehaviour.cpp | 11 -- llvm/lib/MCA/InstrBuilder.cpp | 50 +++---- llvm/lib/Target/RISCV/CMakeLists.txt | 1 - llvm/lib/Target/RISCV/MCA/CMakeLists.txt | 14 -- llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp | 148 -------------------- llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h | 62 --------- llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td | 9 -- llvm/tools/llvm-mca/CodeRegion.cpp | 96 +++---------- llvm/tools/llvm-mca/CodeRegion.h | 83 ++--------- llvm/tools/llvm-mca/CodeRegionGenerator.cpp | 148 ++++++-------------- llvm/tools/llvm-mca/CodeRegionGenerator.h | 152 ++------------------- llvm/tools/llvm-mca/llvm-mca.cpp | 60 ++------ llvm/unittests/tools/llvm-mca/MCATestBase.cpp | 7 +- .../tools/llvm-mca/X86/TestIncrementalMCA.cpp | 14 +- 18 files changed, 130 insertions(+), 970 deletions(-) delete mode 100644 llvm/lib/Target/RISCV/MCA/CMakeLists.txt delete mode 100644 llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp delete mode 100644 llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h diff --git a/llvm/docs/CommandGuide/llvm-mca.rst b/llvm/docs/CommandGuide/llvm-mca.rst index f5f847a..fdb4578 100644 --- a/llvm/docs/CommandGuide/llvm-mca.rst +++ b/llvm/docs/CommandGuide/llvm-mca.rst @@ -227,12 +227,6 @@ option specifies "``-``", then the output will also be sent to standard output. detect any custom hazards or make any post processing modifications to instructions. -.. option:: -disable-im - - Force usage of the generic InstrumentManager rather than using the target - specific implementation. The generic class creates Instruments that provide - no extra information, and InstrumentManager never overrides the default - schedule class for a given instruction. EXIT STATUS ----------- @@ -244,9 +238,9 @@ USING MARKERS TO ANALYZE SPECIFIC CODE BLOCKS --------------------------------------------- :program:`llvm-mca` allows for the optional usage of special code comments to mark regions of the assembly code to be analyzed. A comment starting with -substring ``LLVM-MCA-BEGIN`` marks the beginning of an analysis region. A -comment starting with substring ``LLVM-MCA-END`` marks the end of a region. -For example: +substring ``LLVM-MCA-BEGIN`` marks the beginning of a code region. A comment +starting with substring ``LLVM-MCA-END`` marks the end of a code region. For +example: .. code-block:: none @@ -257,9 +251,9 @@ For example: If no user-defined region is specified, then :program:`llvm-mca` assumes a default region which contains every instruction in the input file. Every region is analyzed in isolation, and the final performance report is the union of all -the reports generated for every analysis region. +the reports generated for every code region. -Analysis regions can have names. For example: +Code regions can have names. For example: .. code-block:: none @@ -321,91 +315,6 @@ assembly is equivalent to the assembly generated in the absence of markers. The `Clang options to emit optimization reports `_ can also help in detecting missed optimizations. -INSTRUMENT REGIONS ------------------- - -An InstrumentRegion describes a region of assembly code guarded by -special LLVM-MCA comment directives. - -.. code-block:: none - - # LLVM-MCA- - ... ## asm - -where `INSTRUMENT_TYPE` is a type defined by the target and expects -to use `data`. - -A comment starting with substring `LLVM-MCA-` -brings data into scope for llvm-mca to use in its analysis for -all following instructions. - -If a comment with the same `INSTRUMENT_TYPE` is found later in the -instruction list, then the original InstrumentRegion will be -automatically ended, and a new InstrumentRegion will begin. - -If there are comments containing the different `INSTRUMENT_TYPE`, -then both data sets remain available. In contrast with an AnalysisRegion, -an InstrumentRegion does not need a comment to end the region. - -Comments that are prefixed with `LLVM-MCA-` but do not correspond to -a valid `INSTRUMENT_TYPE` for the target cause an error, except for -`BEGIN` and `END`, since those correspond to AnalysisRegions. Comments -that do not start with `LLVM-MCA-` are ignored by :program `llvm-mca`. - -An instruction (a MCInst) is added to an InstrumentRegion R only -if its location is in range [R.RangeStart, R.RangeEnd]. - -On RISCV targets, vector instructions have different behaviour depending -on the LMUL. Code can be instrumented with a comment that takes the -following form: - -.. code-block:: none - - # LLVM-MCA-RISCV-LMUL - -The RISCV InstrumentManager will override the schedule class for vector -instructions to use the scheduling behaviour of its pseudo-instruction -which is LMUL dependent. It makes sense to place RISCV instrument -comments directly after `vset{i}vl{i}` instructions, although -they can be placed anywhere in the program. - -Example of program with no call to `vset{i}vl{i}`: - -.. code-block:: none - - # LLVM-MCA-RISCV-LMUL M2 - vadd.vv v2, v2, v2 - -Example of program with call to `vset{i}vl{i}`: - -.. code-block:: none - - vsetvli zero, a0, e8, m1, tu, mu - # LLVM-MCA-RISCV-LMUL M1 - vadd.vv v2, v2, v2 - -Example of program with multiple calls to `vset{i}vl{i}`: - -.. code-block:: none - - vsetvli zero, a0, e8, m1, tu, mu - # LLVM-MCA-RISCV-LMUL M1 - vadd.vv v2, v2, v2 - vsetvli zero, a0, e8, m8, tu, mu - # LLVM-MCA-RISCV-LMUL M8 - vadd.vv v2, v2, v2 - -Example of program with call to `vsetvl`: - -.. code-block:: none - - vsetvl rd, rs1, rs2 - # LLVM-MCA-RISCV-LMUL M1 - vadd.vv v12, v12, v12 - vsetvl rd, rs1, rs2 - # LLVM-MCA-RISCV-LMUL M4 - vadd.vv v12, v12, v12 - HOW LLVM-MCA WORKS ------------------ @@ -1115,28 +1024,6 @@ already have one, refer to an existing implementation to see how to set it up. The classes are implemented within the target specific backend (for example `/llvm/lib/Target/AMDGPU/MCA/`) so that they can access backend symbols. -Instrument Manager -"""""""""""""""""""""""""""""""""""" -On certain architectures, scheduling information for certain instructions -do not contain all of the information required to identify the most precise -schedule class. For example, data that can have an impact on scheduling can -be stored in CSR registers. - -One example of this is on RISCV, where values in registers such as `vtype` -and `vl` change the scheduling behaviour of vector instructions. Since MCA -does not keep track of the values in registers, instrument comments can -be used to specify these values. - -InstrumentManager's main function is `getSchedClassID()` which has access -to the MCInst and all of the instruments that are active for that MCInst. -This function can use the instruments to override the schedule class of -the MCInst. - -On RISCV, instrument comments containing LMUL information are used -by `getSchedClassID()` to map a vector instruction and the active -LMUL to the scheduling class of the pseudo-instruction that describes -that base instruction and the active LMUL. - Custom Views """""""""""""""""""""""""""""""""""" :program:`llvm-mca` comes with several Views such as the Timeline View and diff --git a/llvm/include/llvm/MC/TargetRegistry.h b/llvm/include/llvm/MC/TargetRegistry.h index 1a7b5d2..eeac559 100644 --- a/llvm/include/llvm/MC/TargetRegistry.h +++ b/llvm/include/llvm/MC/TargetRegistry.h @@ -60,7 +60,6 @@ class TargetOptions; namespace mca { class CustomBehaviour; class InstrPostProcess; -class InstrumentManager; struct SourceMgr; } // namespace mca @@ -135,9 +134,6 @@ mca::CustomBehaviour *createCustomBehaviour(const MCSubtargetInfo &STI, mca::InstrPostProcess *createInstrPostProcess(const MCSubtargetInfo &STI, const MCInstrInfo &MCII); -mca::InstrumentManager *createInstrumentManager(const MCSubtargetInfo &STI, - const MCInstrInfo &MCII); - /// Target - Wrapper for Target specific information. /// /// For registration purposes, this is a POD type so that targets can be @@ -249,10 +245,6 @@ public: mca::InstrPostProcess *(*)(const MCSubtargetInfo &STI, const MCInstrInfo &MCII); - using InstrumentManagerCtorTy = - mca::InstrumentManager *(*)(const MCSubtargetInfo &STI, - const MCInstrInfo &MCII); - private: /// Next - The next registered target in the linked list, maintained by the /// TargetRegistry. @@ -362,10 +354,6 @@ private: /// InstrPostProcess, if registered (default = nullptr). InstrPostProcessCtorTy InstrPostProcessCtorFn = nullptr; - /// InstrumentManagerCtorFn - Construction function for this target's - /// InstrumentManager, if registered (default = nullptr). - InstrumentManagerCtorTy InstrumentManagerCtorFn = nullptr; - public: Target() = default; @@ -718,17 +706,6 @@ public: return nullptr; } - /// createInstrumentManager - Create a target specific - /// InstrumentManager. This class is used by llvm-mca and requires - /// backend functionality. - mca::InstrumentManager * - createInstrumentManager(const MCSubtargetInfo &STI, - const MCInstrInfo &MCII) const { - if (InstrumentManagerCtorFn) - return InstrumentManagerCtorFn(STI, MCII); - return nullptr; - } - /// @} }; @@ -1101,21 +1078,6 @@ struct TargetRegistry { T.InstrPostProcessCtorFn = Fn; } - /// RegisterInstrumentManager - Register an InstrumentManager - /// implementation for the given target. - /// - /// Clients are responsible for ensuring that registration doesn't occur - /// while another thread is attempting to access the registry. Typically - /// this is done by initializing all targets at program startup. - /// - /// @param T - The target being registered. - /// @param Fn - A function to construct an InstrumentManager for the - /// target. - static void RegisterInstrumentManager(Target &T, - Target::InstrumentManagerCtorTy Fn) { - T.InstrumentManagerCtorFn = Fn; - } - /// @} }; diff --git a/llvm/include/llvm/MCA/CustomBehaviour.h b/llvm/include/llvm/MCA/CustomBehaviour.h index a74c8db..527dc76 100644 --- a/llvm/include/llvm/MCA/CustomBehaviour.h +++ b/llvm/include/llvm/MCA/CustomBehaviour.h @@ -18,7 +18,6 @@ #ifndef LLVM_MCA_CUSTOMBEHAVIOUR_H #define LLVM_MCA_CUSTOMBEHAVIOUR_H -#include "llvm/ADT/SmallVector.h" #include "llvm/MC/MCInst.h" #include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCSubtargetInfo.h" @@ -115,61 +114,6 @@ public: getEndViews(llvm::MCInstPrinter &IP, llvm::ArrayRef Insts); }; -class Instrument { - /// The description of Instrument kind - const StringRef Desc; - - /// The instrumentation data - const StringRef Data; - -public: - Instrument(StringRef Desc, StringRef Data) : Desc(Desc), Data(Data) {} - - Instrument() = default; - - virtual ~Instrument() = default; - - StringRef getDesc() const { return Desc; } - StringRef getData() const { return Data; } -}; - -using SharedInstrument = std::shared_ptr; - -/// This class allows targets to optionally customize the logic that resolves -/// scheduling class IDs. Targets can use information encoded in Instrument -/// objects to make more informed scheduling decisions. -class InstrumentManager { -protected: - const MCSubtargetInfo &STI; - const MCInstrInfo &MCII; - -public: - InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII) - : STI(STI), MCII(MCII) {} - - virtual ~InstrumentManager() = default; - - /// Returns true if llvm-mca should ignore instruments. - virtual bool shouldIgnoreInstruments() const { return true; } - - // Returns true if this supports processing Instrument with - // Instrument.Desc equal to Type - virtual bool supportsInstrumentType(StringRef Type) const { return false; } - - /// Allocate an Instrument, and return a shared pointer to it. - virtual SharedInstrument createInstrument(StringRef Desc, StringRef Data); - - /// Given an MCInst and a vector of Instrument, a target can - /// return a SchedClassID. This can be used by a subtarget to return a - /// PseudoInstruction SchedClassID instead of the one that belongs to the - /// BaseInstruction This can be useful when a BaseInstruction does not convey - /// the correct scheduling information without additional data. By default, - /// it returns the SchedClassID that belongs to MCI. - virtual unsigned - getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI, - const SmallVector &IVec) const; -}; - } // namespace mca } // namespace llvm diff --git a/llvm/include/llvm/MCA/InstrBuilder.h b/llvm/include/llvm/MCA/InstrBuilder.h index cca71bb..92b92a5 100644 --- a/llvm/include/llvm/MCA/InstrBuilder.h +++ b/llvm/include/llvm/MCA/InstrBuilder.h @@ -19,7 +19,6 @@ #include "llvm/MC/MCInstrInfo.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCSubtargetInfo.h" -#include "llvm/MCA/CustomBehaviour.h" #include "llvm/MCA/Instruction.h" #include "llvm/MCA/Support.h" #include "llvm/Support/Error.h" @@ -63,18 +62,10 @@ class InstrBuilder { const MCInstrInfo &MCII; const MCRegisterInfo &MRI; const MCInstrAnalysis *MCIA; - const InstrumentManager &IM; SmallVector ProcResourceMasks; - // Key is the MCI.Opcode and SchedClassID the describe the value InstrDesc - DenseMap, - std::unique_ptr> - Descriptors; - - // Key is the MCIInst and SchedClassID the describe the value InstrDesc - DenseMap, - std::unique_ptr> - VariantDescriptors; + DenseMap> Descriptors; + DenseMap> VariantDescriptors; bool FirstCallInst; bool FirstReturnInst; @@ -83,12 +74,8 @@ class InstrBuilder { llvm::function_ref; InstRecycleCallback InstRecycleCB; - Expected - createInstrDescImpl(const MCInst &MCI, - const SmallVector &IVec); - Expected - getOrCreateInstrDesc(const MCInst &MCI, - const SmallVector &IVec); + Expected createInstrDescImpl(const MCInst &MCI); + Expected getOrCreateInstrDesc(const MCInst &MCI); InstrBuilder(const InstrBuilder &) = delete; InstrBuilder &operator=(const InstrBuilder &) = delete; @@ -99,8 +86,7 @@ class InstrBuilder { public: InstrBuilder(const MCSubtargetInfo &STI, const MCInstrInfo &MCII, - const MCRegisterInfo &RI, const MCInstrAnalysis *IA, - const InstrumentManager &IM); + const MCRegisterInfo &RI, const MCInstrAnalysis *IA); void clear() { Descriptors.clear(); @@ -113,9 +99,7 @@ public: /// or null if there isn't any. void setInstRecycleCallback(InstRecycleCallback CB) { InstRecycleCB = CB; } - Expected> - createInstruction(const MCInst &MCI, - const SmallVector &IVec); + Expected> createInstruction(const MCInst &MCI); }; } // namespace mca } // namespace llvm diff --git a/llvm/lib/MCA/CustomBehaviour.cpp b/llvm/lib/MCA/CustomBehaviour.cpp index b593e96..a10a2f5 100644 --- a/llvm/lib/MCA/CustomBehaviour.cpp +++ b/llvm/lib/MCA/CustomBehaviour.cpp @@ -42,16 +42,5 @@ CustomBehaviour::getEndViews(llvm::MCInstPrinter &IP, return std::vector>(); } -SharedInstrument InstrumentManager::createInstrument(llvm::StringRef Desc, - llvm::StringRef Data) { - return std::make_shared(Desc, Data); -} - -unsigned InstrumentManager::getSchedClassID( - const MCInstrInfo &MCII, const MCInst &MCI, - const llvm::SmallVector &IVec) const { - return MCII.get(MCI.getOpcode()).getSchedClass(); -} - } // namespace mca } // namespace llvm diff --git a/llvm/lib/MCA/InstrBuilder.cpp b/llvm/lib/MCA/InstrBuilder.cpp index 5a2989a..71c5652 100644 --- a/llvm/lib/MCA/InstrBuilder.cpp +++ b/llvm/lib/MCA/InstrBuilder.cpp @@ -30,9 +30,8 @@ char RecycledInstErr::ID = 0; InstrBuilder::InstrBuilder(const llvm::MCSubtargetInfo &sti, const llvm::MCInstrInfo &mcii, const llvm::MCRegisterInfo &mri, - const llvm::MCInstrAnalysis *mcia, - const mca::InstrumentManager &im) - : STI(sti), MCII(mcii), MRI(mri), MCIA(mcia), IM(im), FirstCallInst(true), + const llvm::MCInstrAnalysis *mcia) + : STI(sti), MCII(mcii), MRI(mri), MCIA(mcia), FirstCallInst(true), FirstReturnInst(true) { const MCSchedModel &SM = STI.getSchedModel(); ProcResourceMasks.resize(SM.getNumProcResourceKinds()); @@ -510,8 +509,7 @@ Error InstrBuilder::verifyInstrDesc(const InstrDesc &ID, } Expected -InstrBuilder::createInstrDescImpl(const MCInst &MCI, - const SmallVector &IVec) { +InstrBuilder::createInstrDescImpl(const MCInst &MCI) { assert(STI.getSchedModel().hasInstrSchedModel() && "Itineraries are not yet supported!"); @@ -521,8 +519,7 @@ InstrBuilder::createInstrDescImpl(const MCInst &MCI, const MCSchedModel &SM = STI.getSchedModel(); // Then obtain the scheduling class information from the instruction. - // Allow InstrumentManager to override and use a different SchedClassID - unsigned SchedClassID = IM.getSchedClassID(MCII, MCI, IVec); + unsigned SchedClassID = MCDesc.getSchedClass(); bool IsVariant = SM.getSchedClassDesc(SchedClassID)->isVariant(); // Try to solve variant scheduling classes. @@ -589,41 +586,30 @@ InstrBuilder::createInstrDescImpl(const MCInst &MCI, // Now add the new descriptor. bool IsVariadic = MCDesc.isVariadic(); if ((ID->IsRecyclable = !IsVariadic && !IsVariant)) { - auto DKey = std::make_pair(MCI.getOpcode(), SchedClassID); - Descriptors[DKey] = std::move(ID); - return *Descriptors[DKey]; + Descriptors[MCI.getOpcode()] = std::move(ID); + return *Descriptors[MCI.getOpcode()]; } - auto VDKey = std::make_pair(&MCI, SchedClassID); - VariantDescriptors[VDKey] = std::move(ID); - return *VariantDescriptors[VDKey]; + VariantDescriptors[&MCI] = std::move(ID); + return *VariantDescriptors[&MCI]; } Expected -InstrBuilder::getOrCreateInstrDesc(const MCInst &MCI, - const SmallVector &IVec) { - // Cache lookup using SchedClassID from Instrumentation - unsigned SchedClassID = IM.getSchedClassID(MCII, MCI, IVec); - - auto DKey = std::make_pair(MCI.getOpcode(), SchedClassID); - if (Descriptors.find_as(DKey) != Descriptors.end()) - return *Descriptors[DKey]; - - unsigned CPUID = STI.getSchedModel().getProcessorID(); - SchedClassID = STI.resolveVariantSchedClass(SchedClassID, &MCI, &MCII, CPUID); - auto VDKey = std::make_pair(&MCI, SchedClassID); - if (VariantDescriptors.find(VDKey) != VariantDescriptors.end()) - return *VariantDescriptors[VDKey]; - - return createInstrDescImpl(MCI, IVec); +InstrBuilder::getOrCreateInstrDesc(const MCInst &MCI) { + if (Descriptors.find_as(MCI.getOpcode()) != Descriptors.end()) + return *Descriptors[MCI.getOpcode()]; + + if (VariantDescriptors.find(&MCI) != VariantDescriptors.end()) + return *VariantDescriptors[&MCI]; + + return createInstrDescImpl(MCI); } STATISTIC(NumVariantInst, "Number of MCInsts that doesn't have static Desc"); Expected> -InstrBuilder::createInstruction(const MCInst &MCI, - const SmallVector &IVec) { - Expected DescOrErr = getOrCreateInstrDesc(MCI, IVec); +InstrBuilder::createInstruction(const MCInst &MCI) { + Expected DescOrErr = getOrCreateInstrDesc(MCI); if (!DescOrErr) return DescOrErr.takeError(); const InstrDesc &D = *DescOrErr; diff --git a/llvm/lib/Target/RISCV/CMakeLists.txt b/llvm/lib/Target/RISCV/CMakeLists.txt index f8e88d5..cbe6e9c 100644 --- a/llvm/lib/Target/RISCV/CMakeLists.txt +++ b/llvm/lib/Target/RISCV/CMakeLists.txt @@ -68,5 +68,4 @@ add_llvm_target(RISCVCodeGen add_subdirectory(AsmParser) add_subdirectory(Disassembler) add_subdirectory(MCTargetDesc) -add_subdirectory(MCA) add_subdirectory(TargetInfo) diff --git a/llvm/lib/Target/RISCV/MCA/CMakeLists.txt b/llvm/lib/Target/RISCV/MCA/CMakeLists.txt deleted file mode 100644 index ca6b78b..0000000 --- a/llvm/lib/Target/RISCV/MCA/CMakeLists.txt +++ /dev/null @@ -1,14 +0,0 @@ -add_llvm_component_library(LLVMRISCVTargetMCA - RISCVCustomBehaviour.cpp - - LINK_COMPONENTS - MC - MCParser - RISCVDesc - RISCVInfo - Support - MCA - - ADD_TO_COMPONENT - RISCV - ) diff --git a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp b/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp deleted file mode 100644 index 277b976..0000000 --- a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.cpp +++ /dev/null @@ -1,148 +0,0 @@ -//===------------------- RISCVCustomBehaviour.cpp ---------------*-C++ -* -===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -/// \file -/// -/// This file implements methods from the RISCVCustomBehaviour class. -/// -//===----------------------------------------------------------------------===// - -#include "RISCVCustomBehaviour.h" -#include "MCTargetDesc/RISCVMCTargetDesc.h" -#include "RISCVInstrInfo.h" -#include "TargetInfo/RISCVTargetInfo.h" -#include "llvm/MC/TargetRegistry.h" -#include "llvm/Support/Debug.h" - -#define DEBUG_TYPE "llvm-mca-riscv-custombehaviour" - -// This brings in a table with primary key of -// base instruction opcode and lmul and maps -// to the opcode of the pseudo instruction. -namespace RISCVVInversePseudosTable { -using namespace llvm; -using namespace llvm::RISCV; - -struct PseudoInfo { - uint16_t Pseudo; - uint16_t BaseInstr; - uint8_t VLMul; -}; - -#define GET_RISCVVInversePseudosTable_IMPL -#define GET_RISCVVInversePseudosTable_DECL -#include "RISCVGenSearchableTables.inc" - -} // end namespace RISCVVInversePseudosTable - -namespace llvm { -namespace mca { - -const llvm::StringRef RISCVLMULInstrument::DESC_NAME = "RISCV-LMUL"; - -bool RISCVLMULInstrument::isDataValid(llvm::StringRef Data) { - // Return true if not one of the valid LMUL strings - return StringSwitch(Data) - .Cases("M1", "M2", "M4", "M8", "MF2", "MF4", "MF8", true) - .Default(false); -} - -uint8_t RISCVLMULInstrument::getLMUL() const { - // assertion prevents us from needing llvm_unreachable in the StringSwitch - // below - assert(isDataValid(getData()) && - "Cannot get LMUL because invalid Data value"); - // These are the LMUL values that are used in RISCV tablegen - return StringSwitch(getData()) - .Case("M1", 0b000) - .Case("M2", 0b001) - .Case("M4", 0b010) - .Case("M8", 0b011) - .Case("MF2", 0b101) - .Case("MF4", 0b110) - .Case("MF8", 0b111); -} - -bool RISCVInstrumentManager::supportsInstrumentType( - llvm::StringRef Type) const { - // Currently, only support for RISCVLMULInstrument type - return Type == RISCVLMULInstrument::DESC_NAME; -} - -SharedInstrument -RISCVInstrumentManager::createInstrument(llvm::StringRef Desc, - llvm::StringRef Data) { - if (Desc != RISCVLMULInstrument::DESC_NAME) { - LLVM_DEBUG(dbgs() << "RVCB: Unknown instrumentation Desc: " << Desc - << '\n'); - return nullptr; - } - if (RISCVLMULInstrument::isDataValid(Data)) { - LLVM_DEBUG(dbgs() << "RVCB: Bad data for instrument kind " << Desc << ": " - << Data << '\n'); - return nullptr; - } - return std::make_shared(Data); -} - -unsigned RISCVInstrumentManager::getSchedClassID( - const MCInstrInfo &MCII, const MCInst &MCI, - const llvm::SmallVector &IVec) const { - unsigned short Opcode = MCI.getOpcode(); - unsigned SchedClassID = MCII.get(Opcode).getSchedClass(); - - for (const auto &I : IVec) { - // Unknown Instrument kind - if (I->getDesc() == RISCVLMULInstrument::DESC_NAME) { - uint8_t LMUL = static_cast(I.get())->getLMUL(); - const RISCVVInversePseudosTable::PseudoInfo *RVV = - RISCVVInversePseudosTable::getBaseInfo(Opcode, LMUL); - // Not a RVV instr - if (!RVV) { - LLVM_DEBUG( - dbgs() - << "RVCB: Could not find PseudoInstruction for Opcode " - << MCII.getName(Opcode) << ", LMUL=" << I->getData() - << ". Ignoring instrumentation and using original SchedClassID=" - << SchedClassID << '\n'); - return SchedClassID; - } - - // Override using pseudo - LLVM_DEBUG(dbgs() << "RVCB: Found Pseudo Instruction for Opcode " - << MCII.getName(Opcode) << ", LMUL=" << I->getData() - << ". Overriding original SchedClassID=" << SchedClassID - << " with " << MCII.getName(RVV->Pseudo) << '\n'); - return MCII.get(RVV->Pseudo).getSchedClass(); - } - } - - // Unknown Instrument kind - LLVM_DEBUG( - dbgs() << "RVCB: Did not use instrumentation to override Opcode.\n"); - return SchedClassID; -} - -} // namespace mca -} // namespace llvm - -using namespace llvm; -using namespace mca; - -static InstrumentManager * -createRISCVInstrumentManager(const MCSubtargetInfo &STI, - const MCInstrInfo &MCII) { - return new RISCVInstrumentManager(STI, MCII); -} - -/// Extern function to initialize the targets for the RISCV backend -extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTargetMCA() { - TargetRegistry::RegisterInstrumentManager(getTheRISCV32Target(), - createRISCVInstrumentManager); - TargetRegistry::RegisterInstrumentManager(getTheRISCV64Target(), - createRISCVInstrumentManager); -} diff --git a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h b/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h deleted file mode 100644 index b3737c9..0000000 --- a/llvm/lib/Target/RISCV/MCA/RISCVCustomBehaviour.h +++ /dev/null @@ -1,62 +0,0 @@ -//===-------------------- RISCVCustomBehaviour.h -----------------*-C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -/// \file -/// -/// This file defines the RISCVCustomBehaviour class which inherits from -/// CustomBehaviour. This class is used by the tool llvm-mca to enforce -/// target specific behaviour that is not expressed well enough in the -/// scheduling model for mca to enforce it automatically. -/// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_LIB_TARGET_RISCV_MCA_RISCVCUSTOMBEHAVIOUR_H -#define LLVM_LIB_TARGET_RISCV_MCA_RISCVCUSTOMBEHAVIOUR_H - -#include "llvm/ADT/SmallVector.h" -#include "llvm/MC/MCInst.h" -#include "llvm/MC/MCInstrDesc.h" -#include "llvm/MC/MCInstrInfo.h" -#include "llvm/MCA/CustomBehaviour.h" - -namespace llvm { -namespace mca { - -class RISCVLMULInstrument : public Instrument { -public: - static const StringRef DESC_NAME; - static bool isDataValid(StringRef Data); - - RISCVLMULInstrument(StringRef Data) : Instrument(DESC_NAME, Data) {} - - ~RISCVLMULInstrument() = default; - - uint8_t getLMUL() const; -}; - -class RISCVInstrumentManager : public InstrumentManager { -public: - RISCVInstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII) - : InstrumentManager(STI, MCII) {} - - bool shouldIgnoreInstruments() const override { return false; } - bool supportsInstrumentType(StringRef Type) const override; - - /// Create a Instrument for RISCV target - SharedInstrument createInstrument(StringRef Desc, StringRef Data) override; - - /// Using the Instrument, returns a SchedClassID to use instead of - /// the SchedClassID that belongs to the MCI or the original SchedClassID. - unsigned - getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI, - const SmallVector &IVec) const override; -}; - -} // namespace mca -} // namespace llvm - -#endif diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td index ee78c0c..0616902 100644 --- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td +++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td @@ -440,15 +440,6 @@ def RISCVVPseudosTable : GenericTable { let PrimaryKeyEarlyOut = true; } -def RISCVVInversePseudosTable : GenericTable { - let FilterClass = "RISCVVPseudo"; - let CppTypeName = "PseudoInfo"; - let Fields = [ "Pseudo", "BaseInstr", "VLMul" ]; - let PrimaryKey = [ "BaseInstr", "VLMul" ]; - let PrimaryKeyName = "getBaseInfo"; - let PrimaryKeyEarlyOut = true; -} - def RISCVVIntrinsicsTable : GenericTable { let FilterClass = "RISCVVIntrinsic"; let CppTypeName = "RISCVVIntrinsicInfo"; diff --git a/llvm/tools/llvm-mca/CodeRegion.cpp b/llvm/tools/llvm-mca/CodeRegion.cpp index 201ee04..7662538 100644 --- a/llvm/tools/llvm-mca/CodeRegion.cpp +++ b/llvm/tools/llvm-mca/CodeRegion.cpp @@ -16,6 +16,11 @@ namespace llvm { namespace mca { +CodeRegions::CodeRegions(llvm::SourceMgr &S) : SM(S), FoundErrors(false) { + // Create a default region for the input code sequence. + Regions.emplace_back(std::make_unique("", SMLoc())); +} + bool CodeRegion::isLocInRange(SMLoc Loc) const { if (RangeEnd.isValid() && Loc.getPointer() > RangeEnd.getPointer()) return false; @@ -24,19 +29,7 @@ bool CodeRegion::isLocInRange(SMLoc Loc) const { return true; } -void CodeRegions::addInstruction(const MCInst &Instruction) { - SMLoc Loc = Instruction.getLoc(); - for (UniqueCodeRegion &Region : Regions) - if (Region->isLocInRange(Loc)) - Region->addInstruction(Instruction); -} - -AnalysisRegions::AnalysisRegions(llvm::SourceMgr &S) : CodeRegions(S) { - // Create a default region for the input code sequence. - Regions.emplace_back(std::make_unique("", SMLoc())); -} - -void AnalysisRegions::beginRegion(StringRef Description, SMLoc Loc) { +void CodeRegions::beginRegion(StringRef Description, SMLoc Loc) { if (ActiveRegions.empty()) { // Remove the default region if there is at least one user defined region. // By construction, only the default region has an invalid start location. @@ -51,17 +44,17 @@ void AnalysisRegions::beginRegion(StringRef Description, SMLoc Loc) { if (It != ActiveRegions.end()) { const CodeRegion &R = *Regions[It->second]; if (Description.empty()) { - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, + SM.PrintMessage(Loc, SourceMgr::DK_Error, "found multiple overlapping anonymous regions"); - SM.PrintMessage(R.startLoc(), llvm::SourceMgr::DK_Note, + SM.PrintMessage(R.startLoc(), SourceMgr::DK_Note, "Previous anonymous region was defined here"); FoundErrors = true; return; } - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, + SM.PrintMessage(Loc, SourceMgr::DK_Error, "overlapping regions cannot have the same name"); - SM.PrintMessage(R.startLoc(), llvm::SourceMgr::DK_Note, + SM.PrintMessage(R.startLoc(), SourceMgr::DK_Note, "region " + Description + " was previously defined here"); FoundErrors = true; return; @@ -72,7 +65,7 @@ void AnalysisRegions::beginRegion(StringRef Description, SMLoc Loc) { Regions.emplace_back(std::make_unique(Description, Loc)); } -void AnalysisRegions::endRegion(StringRef Description, SMLoc Loc) { +void CodeRegions::endRegion(StringRef Description, SMLoc Loc) { if (Description.empty()) { // Special case where there is only one user defined region, // and this LLVM-MCA-END directive doesn't provide a region name. @@ -101,73 +94,22 @@ void AnalysisRegions::endRegion(StringRef Description, SMLoc Loc) { } FoundErrors = true; - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, + SM.PrintMessage(Loc, SourceMgr::DK_Error, "found an invalid region end directive"); if (!Description.empty()) { - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Note, + SM.PrintMessage(Loc, SourceMgr::DK_Note, "unable to find an active region named " + Description); } else { - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Note, + SM.PrintMessage(Loc, SourceMgr::DK_Note, "unable to find an active anonymous region"); } } -InstrumentRegions::InstrumentRegions(llvm::SourceMgr &S) : CodeRegions(S) {} - -void InstrumentRegions::beginRegion(StringRef Description, SMLoc Loc, - SharedInstrument I) { - if (Description.empty()) { - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, - "anonymous instrumentation regions are not permitted"); - FoundErrors = true; - return; - } - - auto It = ActiveRegions.find(Description); - if (It != ActiveRegions.end()) { - const CodeRegion &R = *Regions[It->second]; - SM.PrintMessage( - Loc, llvm::SourceMgr::DK_Error, - "overlapping instrumentation regions cannot be of the same kind"); - SM.PrintMessage(R.startLoc(), llvm::SourceMgr::DK_Note, - "instrumentation region " + Description + - " was previously defined here"); - FoundErrors = true; - return; - } - - ActiveRegions[Description] = Regions.size(); - Regions.emplace_back(std::make_unique(Description, Loc, I)); -} - -void InstrumentRegions::endRegion(StringRef Description, SMLoc Loc) { - auto It = ActiveRegions.find(Description); - if (It != ActiveRegions.end()) { - Regions[It->second]->setEndLocation(Loc); - ActiveRegions.erase(It); - return; - } - - FoundErrors = true; - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, - "found an invalid instrumentation region end directive"); - if (!Description.empty()) { - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Note, - "unable to find an active instrumentation region named " + - Description); - } -} - -const SmallVector -InstrumentRegions::getActiveInstruments(SMLoc Loc) const { - SmallVector AI; - for (auto &R : Regions) { - if (R->isLocInRange(Loc)) { - InstrumentRegion *IR = static_cast(R.get()); - AI.emplace_back(IR->getInstrument()); - } - } - return AI; +void CodeRegions::addInstruction(const MCInst &Instruction) { + SMLoc Loc = Instruction.getLoc(); + for (UniqueCodeRegion &Region : Regions) + if (Region->isLocInRange(Loc)) + Region->addInstruction(Instruction); } } // namespace mca diff --git a/llvm/tools/llvm-mca/CodeRegion.h b/llvm/tools/llvm-mca/CodeRegion.h index b5b2f3a..0e1e02a 100644 --- a/llvm/tools/llvm-mca/CodeRegion.h +++ b/llvm/tools/llvm-mca/CodeRegion.h @@ -7,8 +7,7 @@ //===----------------------------------------------------------------------===// /// \file /// -/// This file implements class CodeRegion and CodeRegions, InstrumentRegion, -/// AnalysisRegions, and InstrumentRegions. +/// This file implements class CodeRegion and CodeRegions. /// /// A CodeRegion describes a region of assembly code guarded by special LLVM-MCA /// comment directives. @@ -26,32 +25,8 @@ /// description; internally, regions are described by a range of source /// locations (SMLoc objects). /// -/// An instruction (a MCInst) is added to a CodeRegion R only if its -/// location is in range [R.RangeStart, R.RangeEnd]. -/// -/// A InstrumentRegion describes a region of assembly code guarded by -/// special LLVM-MCA comment directives. -/// -/// # LLVM-MCA- -/// ... ## asm -/// -/// where INSTRUMENTATION_TYPE is a type defined in llvm and expects to use -/// data. -/// -/// A comment starting with substring LLVM-MCA- -/// brings data into scope for llvm-mca to use in its analysis for -/// all following instructions. -/// -/// If the same INSTRUMENTATION_TYPE is found later in the instruction list, -/// then the original InstrumentRegion will be automatically ended, -/// and a new InstrumentRegion will begin. -/// -/// If there are comments containing the different INSTRUMENTATION_TYPEs, -/// then both data sets remain available. In contrast with a CodeRegion, -/// an InstrumentRegion does not need a comment to end the region. -// -// An instruction (a MCInst) is added to an InstrumentRegion R only -// if its location is in range [R.RangeStart, R.RangeEnd]. +/// An instruction (a MCInst) is added to a region R only if its location is in +/// range [R.RangeStart, R.RangeEnd]. // //===----------------------------------------------------------------------===// @@ -63,7 +38,6 @@ #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/MC/MCInst.h" -#include "llvm/MCA/CustomBehaviour.h" #include "llvm/Support/Error.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/SourceMgr.h" @@ -107,31 +81,9 @@ public: llvm::StringRef getDescription() const { return Description; } }; -/// Alias AnalysisRegion with CodeRegion since CodeRegionGenerator -/// is absract and AnalysisRegionGenerator operates on AnalysisRegions -using AnalysisRegion = CodeRegion; - -/// A CodeRegion that contains instrumentation that can be used -/// in analysis of the region. -class InstrumentRegion : public CodeRegion { - /// Instrument for this region. - SharedInstrument Instrument; - -public: - InstrumentRegion(llvm::StringRef Desc, llvm::SMLoc Start, SharedInstrument I) - : CodeRegion(Desc, Start), Instrument(I) {} - -public: - SharedInstrument getInstrument() const { return Instrument; } -}; - class CodeRegionParseError final : public Error {}; class CodeRegions { - CodeRegions(const CodeRegions &) = delete; - CodeRegions &operator=(const CodeRegions &) = delete; - -protected: // A source manager. Used by the tool to generate meaningful warnings. llvm::SourceMgr &SM; @@ -140,8 +92,11 @@ protected: llvm::StringMap ActiveRegions; bool FoundErrors; + CodeRegions(const CodeRegions &) = delete; + CodeRegions &operator=(const CodeRegions &) = delete; + public: - CodeRegions(llvm::SourceMgr &S) : SM(S), FoundErrors(false) {} + CodeRegions(llvm::SourceMgr &S); typedef std::vector::iterator iterator; typedef std::vector::const_iterator const_iterator; @@ -151,6 +106,8 @@ public: const_iterator begin() const { return Regions.cbegin(); } const_iterator end() const { return Regions.cend(); } + void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc); + void endRegion(llvm::StringRef Description, llvm::SMLoc Loc); void addInstruction(const llvm::MCInst &Instruction); llvm::SourceMgr &getSourceMgr() const { return SM; } @@ -165,28 +122,6 @@ public: } bool isValid() const { return !FoundErrors; } - - bool isRegionActive(llvm::StringRef Description) const { - return ActiveRegions.find(Description) != ActiveRegions.end(); - } -}; - -struct AnalysisRegions : public CodeRegions { - AnalysisRegions(llvm::SourceMgr &S); - - void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc); - void endRegion(llvm::StringRef Description, llvm::SMLoc Loc); -}; - -struct InstrumentRegions : public CodeRegions { - InstrumentRegions(llvm::SourceMgr &S); - - void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc, - SharedInstrument Instrument); - void endRegion(llvm::StringRef Description, llvm::SMLoc Loc); - - const SmallVector - getActiveInstruments(llvm::SMLoc Loc) const; }; } // namespace mca diff --git a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp index d643234..cdecfba 100644 --- a/llvm/tools/llvm-mca/CodeRegionGenerator.cpp +++ b/llvm/tools/llvm-mca/CodeRegionGenerator.cpp @@ -16,6 +16,7 @@ #include "CodeRegionGenerator.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" +#include "llvm/MC/MCParser/MCAsmLexer.h" #include "llvm/MC/MCParser/MCTargetAsmParser.h" #include "llvm/MC/MCStreamer.h" #include "llvm/MC/MCTargetOptions.h" @@ -29,6 +30,15 @@ namespace mca { // This virtual dtor serves as the anchor for the CodeRegionGenerator class. CodeRegionGenerator::~CodeRegionGenerator() {} +// A comment consumer that parses strings. The only valid tokens are strings. +class MCACommentConsumer : public AsmCommentConsumer { +public: + CodeRegions &Regions; + + MCACommentConsumer(CodeRegions &R) : Regions(R) {} + void HandleComment(SMLoc Loc, StringRef CommentText) override; +}; + // This class provides the callbacks that occur when parsing input assembly. class MCStreamerWrapper final : public MCStreamer { CodeRegions &Regions; @@ -63,53 +73,7 @@ public: } }; -Expected AsmCodeRegionGenerator::parseCodeRegions( - const std::unique_ptr &IP) { - MCTargetOptions Opts; - Opts.PreserveAsmComments = false; - CodeRegions &Regions = getRegions(); - MCStreamerWrapper Str(Ctx, Regions); - - // Need to initialize an MCTargetStreamer otherwise - // certain asm directives will cause a segfault. - // Using nulls() so that anything emitted by the MCTargetStreamer - // doesn't show up in the llvm-mca output. - raw_ostream &OSRef = nulls(); - formatted_raw_ostream FOSRef(OSRef); - TheTarget.createAsmTargetStreamer(Str, FOSRef, IP.get(), - /*IsVerboseAsm=*/true); - - // Create a MCAsmParser and setup the lexer to recognize llvm-mca ASM - // comments. - std::unique_ptr Parser( - createMCAsmParser(Regions.getSourceMgr(), Ctx, Str, MAI)); - MCAsmLexer &Lexer = Parser->getLexer(); - MCACommentConsumer *CCP = getCommentConsumer(); - Lexer.setCommentConsumer(CCP); - // Enable support for MASM literal numbers (example: 05h, 101b). - Lexer.setLexMasmIntegers(true); - - std::unique_ptr TAP( - TheTarget.createMCAsmParser(STI, *Parser, MCII, Opts)); - if (!TAP) - return make_error( - "This target does not support assembly parsing.", - inconvertibleErrorCode()); - Parser->setTargetParser(*TAP); - Parser->Run(false); - - if (CCP->hadErr()) - return make_error("There was an error parsing comments.", - inconvertibleErrorCode()); - - // Set the assembler dialect from the input. llvm-mca will use this as the - // default dialect when printing reports. - AssemblerDialect = Parser->getAssemblerDialect(); - return Regions; -} - -void AnalysisRegionCommentConsumer::HandleComment(SMLoc Loc, - StringRef CommentText) { +void MCACommentConsumer::HandleComment(SMLoc Loc, StringRef CommentText) { // Skip empty comments. StringRef Comment(CommentText); if (Comment.empty()) @@ -143,66 +107,44 @@ void AnalysisRegionCommentConsumer::HandleComment(SMLoc Loc, Regions.beginRegion(Comment, Loc); } -void InstrumentRegionCommentConsumer::HandleComment(SMLoc Loc, - StringRef CommentText) { - // Skip empty comments. - StringRef Comment(CommentText); - if (Comment.empty()) - return; - - // Skip spaces and tabs. - unsigned Position = Comment.find_first_not_of(" \t"); - if (Position >= Comment.size()) - // We reached the end of the comment. Bail out. - return; - Comment = Comment.drop_front(Position); - - // Bail out if not an MCA style comment - if (!Comment.consume_front("LLVM-MCA-")) - return; - - // Skip AnalysisRegion comments - if (Comment.consume_front("BEGIN") || Comment.consume_front("END")) - return; +Expected AsmCodeRegionGenerator::parseCodeRegions( + const std::unique_ptr &IP) { + MCTargetOptions Opts; + Opts.PreserveAsmComments = false; + MCStreamerWrapper Str(Ctx, Regions); - if (IM.shouldIgnoreInstruments()) - return; + // Need to initialize an MCTargetStreamer otherwise + // certain asm directives will cause a segfault. + // Using nulls() so that anything emitted by the MCTargetStreamer + // doesn't show up in the llvm-mca output. + raw_ostream &OSRef = nulls(); + formatted_raw_ostream FOSRef(OSRef); + TheTarget.createAsmTargetStreamer(Str, FOSRef, IP.get(), + /*IsVerboseAsm=*/true); - auto [InstrumentKind, Data] = Comment.split(" "); - - // An error if not of the form LLVM-MCA-TARGET-KIND - if (!IM.supportsInstrumentType(InstrumentKind)) { - if (InstrumentKind.empty()) - SM.PrintMessage( - Loc, llvm::SourceMgr::DK_Error, - "No instrumentation kind was provided in LLVM-MCA comment"); - else - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, - "Unknown instrumentation type in LLVM-MCA comment: " + - InstrumentKind); - FoundError = true; - return; - } + // Create a MCAsmParser and setup the lexer to recognize llvm-mca ASM + // comments. + std::unique_ptr Parser( + createMCAsmParser(Regions.getSourceMgr(), Ctx, Str, MAI)); + MCAsmLexer &Lexer = Parser->getLexer(); + MCACommentConsumer CC(Regions); + Lexer.setCommentConsumer(&CC); + // Enable support for MASM literal numbers (example: 05h, 101b). + Lexer.setLexMasmIntegers(true); - SharedInstrument I = IM.createInstrument(InstrumentKind, Data); - if (!I) { - if (Data.empty()) - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, - "Failed to create " + InstrumentKind + - " instrument with no data"); - else - SM.PrintMessage(Loc, llvm::SourceMgr::DK_Error, - "Failed to create " + InstrumentKind + - " instrument with data: " + Data); - FoundError = true; - return; - } + std::unique_ptr TAP( + TheTarget.createMCAsmParser(STI, *Parser, MCII, Opts)); + if (!TAP) + return make_error( + "This target does not support assembly parsing.", + inconvertibleErrorCode()); + Parser->setTargetParser(*TAP); + Parser->Run(false); - // End InstrumentType region if one is open - if (Regions.isRegionActive(InstrumentKind)) - Regions.endRegion(InstrumentKind, Loc); - // Start new instrumentation region - Regions.beginRegion(InstrumentKind, Loc, I); + // Set the assembler dialect from the input. llvm-mca will use this as the + // default dialect when printing reports. + AssemblerDialect = Parser->getAssemblerDialect(); + return Regions; } } // namespace mca diff --git a/llvm/tools/llvm-mca/CodeRegionGenerator.h b/llvm/tools/llvm-mca/CodeRegionGenerator.h index d9e9be2..ac02131 100644 --- a/llvm/tools/llvm-mca/CodeRegionGenerator.h +++ b/llvm/tools/llvm-mca/CodeRegionGenerator.h @@ -19,10 +19,8 @@ #include "CodeRegion.h" #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" -#include "llvm/MC/MCParser/MCAsmLexer.h" #include "llvm/MC/MCSubtargetInfo.h" #include "llvm/MC/TargetRegistry.h" -#include "llvm/MCA/CustomBehaviour.h" #include "llvm/Support/Error.h" #include "llvm/Support/SourceMgr.h" #include @@ -30,96 +28,24 @@ namespace llvm { namespace mca { -class MCACommentConsumer : public AsmCommentConsumer { -protected: - bool FoundError; - -public: - MCACommentConsumer() : FoundError(false) {} - - bool hadErr() const { return FoundError; } -}; - -/// A comment consumer that parses strings. The only valid tokens are strings. -class AnalysisRegionCommentConsumer : public MCACommentConsumer { - AnalysisRegions &Regions; - -public: - AnalysisRegionCommentConsumer(AnalysisRegions &R) : Regions(R) {} - - /// Parses a comment. It begins a new region if it is of the form - /// LLVM-MCA-BEGIN. It ends a region if it is of the form LLVM-MCA-END. - /// Regions can be optionally named if they are of the form - /// LLVM-MCA-BEGIN or LLVM-MCA-END . Subregions are - /// permitted, but a region that begins while another region is active - /// must be ended before the outer region is ended. If thre is only one - /// active region, LLVM-MCA-END does not need to provide a name. - void HandleComment(SMLoc Loc, StringRef CommentText) override; -}; - -/// A comment consumer that parses strings to create InstrumentRegions. -/// The only valid tokens are strings. -class InstrumentRegionCommentConsumer : public MCACommentConsumer { - llvm::SourceMgr &SM; - - InstrumentRegions &Regions; - - InstrumentManager &IM; - -public: - InstrumentRegionCommentConsumer(llvm::SourceMgr &SM, InstrumentRegions &R, - InstrumentManager &IM) - : SM(SM), Regions(R), IM(IM) {} - - /// Parses a comment. It begins a new region if it is of the form - /// LLVM-MCA- where INSTRUMENTATION_TYPE - /// is a valid InstrumentKind. If there is already an active - /// region of type INSTRUMENATION_TYPE, then it will end the active - /// one and begin a new one using the new data. - void HandleComment(SMLoc Loc, StringRef CommentText) override; -}; - -/// This abstract class is responsible for parsing the input given to -/// the llvm-mca driver, and converting that into a CodeRegions instance. +/// This class is responsible for parsing the input given to the llvm-mca +/// driver, and converting that into a CodeRegions instance. class CodeRegionGenerator { protected: + CodeRegions Regions; CodeRegionGenerator(const CodeRegionGenerator &) = delete; CodeRegionGenerator &operator=(const CodeRegionGenerator &) = delete; - virtual Expected - parseCodeRegions(const std::unique_ptr &IP) = 0; public: - CodeRegionGenerator() {} + CodeRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {} virtual ~CodeRegionGenerator(); + virtual Expected + parseCodeRegions(const std::unique_ptr &IP) = 0; }; -/// Abastract CodeRegionGenerator with AnalysisRegions member -class AnalysisRegionGenerator : public virtual CodeRegionGenerator { -protected: - AnalysisRegions Regions; - -public: - AnalysisRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {} - - virtual Expected - parseAnalysisRegions(const std::unique_ptr &IP) = 0; -}; - -/// Abstract CodeRegionGenerator with InstrumentRegionsRegions member -class InstrumentRegionGenerator : public virtual CodeRegionGenerator { -protected: - InstrumentRegions Regions; - -public: - InstrumentRegionGenerator(llvm::SourceMgr &SM) : Regions(SM) {} - - virtual Expected - parseInstrumentRegions(const std::unique_ptr &IP) = 0; -}; - -/// This abstract class is responsible for parsing input ASM and -/// generating a CodeRegions instance. -class AsmCodeRegionGenerator : public virtual CodeRegionGenerator { +/// This class is responsible for parsing input ASM and generating +/// a CodeRegions instance. +class AsmCodeRegionGenerator final : public CodeRegionGenerator { const Target &TheTarget; MCContext &Ctx; const MCAsmInfo &MAI; @@ -128,67 +54,17 @@ class AsmCodeRegionGenerator : public virtual CodeRegionGenerator { unsigned AssemblerDialect; // This is set during parsing. public: - AsmCodeRegionGenerator(const Target &T, MCContext &C, const MCAsmInfo &A, - const MCSubtargetInfo &S, const MCInstrInfo &I) - : TheTarget(T), Ctx(C), MAI(A), STI(S), MCII(I), AssemblerDialect(0) {} - - virtual MCACommentConsumer *getCommentConsumer() = 0; - virtual CodeRegions &getRegions() = 0; + AsmCodeRegionGenerator(const Target &T, llvm::SourceMgr &SM, MCContext &C, + const MCAsmInfo &A, const MCSubtargetInfo &S, + const MCInstrInfo &I) + : CodeRegionGenerator(SM), TheTarget(T), Ctx(C), MAI(A), STI(S), MCII(I), + AssemblerDialect(0) {} unsigned getAssemblerDialect() const { return AssemblerDialect; } Expected parseCodeRegions(const std::unique_ptr &IP) override; }; -class AsmAnalysisRegionGenerator final : public AnalysisRegionGenerator, - public AsmCodeRegionGenerator { - AnalysisRegionCommentConsumer CC; - -public: - AsmAnalysisRegionGenerator(const Target &T, llvm::SourceMgr &SM, MCContext &C, - const MCAsmInfo &A, const MCSubtargetInfo &S, - const MCInstrInfo &I) - : AnalysisRegionGenerator(SM), AsmCodeRegionGenerator(T, C, A, S, I), - CC(Regions) {} - - MCACommentConsumer *getCommentConsumer() override { return &CC; }; - CodeRegions &getRegions() override { return Regions; }; - - Expected - parseAnalysisRegions(const std::unique_ptr &IP) override { - Expected RegionsOrErr = parseCodeRegions(IP); - if (!RegionsOrErr) - return RegionsOrErr.takeError(); - else - return static_cast(*RegionsOrErr); - } -}; - -class AsmInstrumentRegionGenerator final : public InstrumentRegionGenerator, - public AsmCodeRegionGenerator { - InstrumentRegionCommentConsumer CC; - -public: - AsmInstrumentRegionGenerator(const Target &T, llvm::SourceMgr &SM, - MCContext &C, const MCAsmInfo &A, - const MCSubtargetInfo &S, const MCInstrInfo &I, - InstrumentManager &IM) - : InstrumentRegionGenerator(SM), AsmCodeRegionGenerator(T, C, A, S, I), - CC(SM, Regions, IM) {} - - MCACommentConsumer *getCommentConsumer() override { return &CC; }; - CodeRegions &getRegions() override { return Regions; }; - - Expected - parseInstrumentRegions(const std::unique_ptr &IP) override { - Expected RegionsOrErr = parseCodeRegions(IP); - if (!RegionsOrErr) - return RegionsOrErr.takeError(); - else - return static_cast(*RegionsOrErr); - } -}; - } // namespace mca } // namespace llvm diff --git a/llvm/tools/llvm-mca/llvm-mca.cpp b/llvm/tools/llvm-mca/llvm-mca.cpp index 2a27fea..6f7b74f 100644 --- a/llvm/tools/llvm-mca/llvm-mca.cpp +++ b/llvm/tools/llvm-mca/llvm-mca.cpp @@ -231,12 +231,6 @@ static cl::opt DisableCustomBehaviour( "Disable custom behaviour (use the default class which does nothing)."), cl::cat(ViewOptions), cl::init(false)); -static cl::opt DisableInstrumentManager( - "disable-im", - cl::desc("Disable instrumentation manager (use the default class which " - "ignores instruments.)."), - cl::cat(ViewOptions), cl::init(false)); - namespace { const Target *getTarget(const char *ProgName) { @@ -413,7 +407,7 @@ int main(int argc, char **argv) { // Need to initialize an MCInstPrinter as it is // required for initializing the MCTargetStreamer - // which needs to happen within the CRG.parseAnalysisRegions() call below. + // which needs to happen within the CRG.parseCodeRegions() call below. // Without an MCTargetStreamer, certain assembly directives can trigger a // segfault. (For example, the .cv_fpo_proc directive on x86 will segfault if // we don't initialize the MCTargetStreamer.) @@ -430,10 +424,9 @@ int main(int argc, char **argv) { } // Parse the input and create CodeRegions that llvm-mca can analyze. - mca::AsmAnalysisRegionGenerator CRG(*TheTarget, SrcMgr, Ctx, *MAI, *STI, - *MCII); - Expected RegionsOrErr = - CRG.parseAnalysisRegions(std::move(IPtemp)); + mca::AsmCodeRegionGenerator CRG(*TheTarget, SrcMgr, Ctx, *MAI, *STI, *MCII); + Expected RegionsOrErr = + CRG.parseCodeRegions(std::move(IPtemp)); if (!RegionsOrErr) { if (auto Err = handleErrors(RegionsOrErr.takeError(), [](const StringError &E) { @@ -444,7 +437,7 @@ int main(int argc, char **argv) { } return 1; } - const mca::AnalysisRegions &Regions = *RegionsOrErr; + const mca::CodeRegions &Regions = *RegionsOrErr; // Early exit if errors were found by the code region parsing logic. if (!Regions.isValid()) @@ -455,39 +448,6 @@ int main(int argc, char **argv) { return 1; } - std::unique_ptr IM; - if (!DisableInstrumentManager) { - IM = std::unique_ptr( - TheTarget->createInstrumentManager(*STI, *MCII)); - } - if (!IM) { - // If the target doesn't have its own IM implemented (or the -disable-cb - // flag is set) then we use the base class (which does nothing). - IM = std::make_unique(*STI, *MCII); - } - - // Parse the input and create InstrumentRegion that llvm-mca - // can use to improve analysis. - mca::AsmInstrumentRegionGenerator IRG(*TheTarget, SrcMgr, Ctx, *MAI, *STI, - *MCII, *IM); - Expected InstrumentRegionsOrErr = - IRG.parseInstrumentRegions(std::move(IPtemp)); - if (!InstrumentRegionsOrErr) { - if (auto Err = handleErrors(InstrumentRegionsOrErr.takeError(), - [](const StringError &E) { - WithColor::error() << E.getMessage() << '\n'; - })) { - // Default case. - WithColor::error() << toString(std::move(Err)) << '\n'; - } - return 1; - } - const mca::InstrumentRegions &InstrumentRegions = *InstrumentRegionsOrErr; - - // Early exit if errors were found by the instrumentation parsing logic. - if (!InstrumentRegions.isValid()) - return 1; - // Now initialize the output file. auto OF = getOutputStream(); if (std::error_code EC = OF.getError()) { @@ -531,7 +491,7 @@ int main(int argc, char **argv) { } // Create an instruction builder. - mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM); + mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get()); // Create a context to control ownership of the pipeline hardware. mca::Context MCA(*MRI, *STI); @@ -552,7 +512,7 @@ int main(int argc, char **argv) { assert(MAB && "Unable to create asm backend!"); json::Object JSONOutput; - for (const std::unique_ptr &Region : Regions) { + for (const std::unique_ptr &Region : Regions) { // Skip empty code regions. if (Region->empty()) continue; @@ -567,12 +527,8 @@ int main(int argc, char **argv) { SmallVector> LoweredSequence; for (const MCInst &MCI : Insts) { - SMLoc Loc = MCI.getLoc(); - const SmallVector Instruments = - InstrumentRegions.getActiveInstruments(Loc); - Expected> Inst = - IB.createInstruction(MCI, Instruments); + IB.createInstruction(MCI); if (!Inst) { if (auto NewE = handleErrors( Inst.takeError(), diff --git a/llvm/unittests/tools/llvm-mca/MCATestBase.cpp b/llvm/unittests/tools/llvm-mca/MCATestBase.cpp index 543edbf..0be6f39 100644 --- a/llvm/unittests/tools/llvm-mca/MCATestBase.cpp +++ b/llvm/unittests/tools/llvm-mca/MCATestBase.cpp @@ -64,15 +64,12 @@ Error MCATestBase::runBaselineMCA(json::Object &Result, ArrayRef Insts, const mca::PipelineOptions *PO) { mca::Context MCA(*MRI, *STI); - // Default InstrumentManager - auto IM = std::make_unique(*STI, *MCII); - mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM); + mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get()); - const SmallVector Instruments; SmallVector> LoweredInsts; for (const auto &MCI : Insts) { Expected> Inst = - IB.createInstruction(MCI, Instruments); + IB.createInstruction(MCI); if (!Inst) { if (auto NewE = handleErrors(Inst.takeError(), diff --git a/llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp b/llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp index a0b743e..bcc6e7c 100644 --- a/llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp +++ b/llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp @@ -32,15 +32,13 @@ TEST_F(X86TestBase, TestResumablePipeline) { PO.DispatchWidth); P->addEventListener(SV.get()); - auto IM = std::make_unique(*STI, *MCII); - mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM); + mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get()); - const SmallVector Instruments; // Tile size = 7 for (unsigned i = 0U, E = MCIs.size(); i < E;) { for (unsigned TE = i + 7; i < TE && i < E; ++i) { Expected> InstOrErr = - IB.createInstruction(MCIs[i], Instruments); + IB.createInstruction(MCIs[i]); ASSERT_TRUE(bool(InstOrErr)); ISM.addInst(std::move(InstOrErr.get())); } @@ -121,18 +119,14 @@ TEST_F(X86TestBase, TestInstructionRecycling) { PO.DispatchWidth); P->addEventListener(SV.get()); - // Default InstrumentManager - auto IM = std::make_unique(*STI, *MCII); - - mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM); + mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get()); IB.setInstRecycleCallback(GetRecycledInst); - const SmallVector Instruments; // Tile size = 7 for (unsigned i = 0U, E = MCIs.size(); i < E;) { for (unsigned TE = i + 7; i < TE && i < E; ++i) { Expected> InstOrErr = - IB.createInstruction(MCIs[i], Instruments); + IB.createInstruction(MCIs[i]); if (!InstOrErr) { mca::Instruction *RecycledInst = nullptr; -- 2.7.4