Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
b1a67b7
Added interface to customize individual instructions and possibility …
r-belenov Aug 26, 2025
4abee0f
Fix formatting
r-belenov Aug 26, 2025
1298dd3
Fix formatting
r-belenov Aug 26, 2025
b5c3b95
Fix formatting
r-belenov Aug 26, 2025
302b994
Fix formatting
r-belenov Aug 26, 2025
2338978
Fix formatting
r-belenov Aug 26, 2025
0be9fbf
Fix formatting
r-belenov Aug 26, 2025
f1ad060
Fix formatting
r-belenov Aug 26, 2025
757fec3
Fix formatting
r-belenov Aug 26, 2025
452bb40
Fix formatting
r-belenov Aug 26, 2025
4d655f8
Fix formatting
r-belenov Aug 26, 2025
ef2a0f0
Fix formatting
r-belenov Aug 26, 2025
77eefdb
Fix formatting
r-belenov Aug 26, 2025
d44bfbe
Merge branch 'main' into mca-instruction-customizer
r-belenov Aug 26, 2025
bd8cedf
Merge branch 'main' into mca-instruction-customizer
r-belenov Aug 27, 2025
6e0d12b
Using function_ref to pass function arguments
r-belenov Aug 27, 2025
08cc266
Use InstrumentManager to customize instructions
r-belenov Aug 27, 2025
1282e70
Fix formatting
r-belenov Aug 27, 2025
71403fb
Move latency customization logic to base Instrument
r-belenov Aug 28, 2025
b805a3c
Simplify IM creation
r-belenov Aug 28, 2025
801b0da
Fix formatting
r-belenov Aug 28, 2025
ed4e0bd
Remove target IM from base class
r-belenov Aug 29, 2025
042b3ba
Fix formatting
r-belenov Aug 29, 2025
5fb0db7
Use explicit latency instrument
r-belenov Sep 12, 2025
e124416
Fix formatting (#12)
r-belenov Sep 12, 2025
74482dd
Removing redundant include
r-belenov Sep 12, 2025
37d0734
Merge branch 'main' into mca-instruction-customizer
r-belenov Sep 12, 2025
5824130
Make latency test more consistent
r-belenov Sep 12, 2025
5029c24
Merge branch 'main' into mca-instruction-customizer
r-belenov Sep 15, 2025
7c40f7b
Move customization logic to InstrumentManager
r-belenov Sep 17, 2025
9e276b6
Fix formatting (#14)
r-belenov Sep 17, 2025
129b7e4
Merge branch 'main' into mca-instruction-customizer
r-belenov Sep 17, 2025
e28c48c
FIxed incorrect Enlish
r-belenov Sep 18, 2025
7e90de7
Addressed minor comments
r-belenov Sep 18, 2025
4065142
Fixed formatting
r-belenov Sep 18, 2025
f8113c4
Merge branch 'main' into mca-instruction-customizer
r-belenov Sep 18, 2025
6043f89
Ensure test instruction can not be eliminated on dispatch stage
r-belenov Sep 18, 2025
0d00426
Merge branch 'main' into mca-instruction-customizer
r-belenov Sep 18, 2025
7bf43cf
Revert to explicit StringRef conversion to check the tests
r-belenov Sep 18, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions llvm/include/llvm/MCA/CustomBehaviour.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ class Instrument {

virtual ~Instrument() = default;

virtual bool canCustomize() const { return false; }
virtual void customize(InstrDesc &) const {}

StringRef getDesc() const { return Desc; }
StringRef getData() const { return Data; }
};
Expand All @@ -143,19 +146,27 @@ class LLVM_ABI InstrumentManager {
protected:
const MCSubtargetInfo &STI;
const MCInstrInfo &MCII;
bool EnableDefaults;
std::unique_ptr<InstrumentManager> TargetIM;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InstrumentManager is already target-specific, why do you need TargetIM here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have InstrDesc customization generic (e.g. latency change code is generic, other things I do locally also do not require target details) and coexist with target-specific instrumentation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have InstrDesc customization generic (e.g. latency change code is generic, other things I do locally also do not require target details) and coexist with target-specific instrumentation.

I think my concern here is that this additional layer of pointer indirection could be avoid if we could simply put this latency updating logics in the base IM and called it from the target-specific, derived IM when needed, as I described in the sibling comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now the customization in InstrumentManager class and can be reused.


public:
InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII)
: STI(STI), MCII(MCII) {}
InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII,
bool EnableDefaults = false,
std::unique_ptr<InstrumentManager> TargetIM = {})
: STI(STI), MCII(MCII), EnableDefaults(EnableDefaults),
TargetIM(std::move(TargetIM)) {}

virtual ~InstrumentManager() = default;

/// Returns true if llvm-mca should ignore instruments.
virtual bool shouldIgnoreInstruments() const { return true; }
virtual bool shouldIgnoreInstruments() const {
return !EnableDefaults &&
(!TargetIM || TargetIM->shouldIgnoreInstruments());
}

// Returns true if this supports processing Instrument with
// Instrument.Desc equal to Type
virtual bool supportsInstrumentType(StringRef Type) const { return false; }
virtual bool supportsInstrumentType(StringRef Type) const;

/// Allocate an Instrument, and return a unique pointer to it. This function
/// may be useful to create instruments coming from comments in the assembly.
Expand All @@ -175,6 +186,13 @@ class LLVM_ABI InstrumentManager {
/// it returns the SchedClassID that belongs to MCI.
virtual unsigned getSchedClassID(const MCInstrInfo &MCII, const MCInst &MCI,
const SmallVector<Instrument *> &IVec) const;

// Return true if instruments can modify instruction description
virtual bool canCustomize(const SmallVector<Instrument *> &IVec) const;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use ArrayRef here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Makes sense for getSchedClassID too; didn't touch it since it would break existing API.


// Customize instruction description
virtual void customize(const SmallVector<Instrument *> &IVec,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto ArrayRef for IVec

llvm::mca::InstrDesc &Desc) const;
};

} // namespace mca
Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/MCA/InstrBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "llvm/MCA/Support.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/Error.h"
#include <functional>

namespace llvm {
namespace mca {
Expand Down Expand Up @@ -78,6 +79,10 @@ class InstrBuilder {
DenseMap<std::pair<hash_code, unsigned>, std::unique_ptr<const InstrDesc>>
VariantDescriptors;

// These descriptors are customized for particular instructions and cannot
// be reused
SmallVector<std::unique_ptr<const InstrDesc>> CustomDescriptors;

bool FirstCallInst;
bool FirstReturnInst;
unsigned CallLatency;
Expand Down
73 changes: 73 additions & 0 deletions llvm/lib/MCA/CustomBehaviour.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//

#include "llvm/MCA/CustomBehaviour.h"
#include "llvm/MCA/Instruction.h"

namespace llvm {
namespace mca {
Expand Down Expand Up @@ -42,19 +43,91 @@ CustomBehaviour::getEndViews(llvm::MCInstPrinter &IP,
return std::vector<std::unique_ptr<View>>();
}

class CustomInstrument : public Instrument {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be solved in a much simpler way: given a line "LLVM-MCA-LATENCY: 100", the default Instrument could already provide you Desc and Data of "LLVM-MCA-LATENCY:" and "100" respectively. So I don't think you need the CustomInstrument here.
And then for supportsInstrumentType, canCustomize and customize -- you could just put the implementation directly in the function, and call this base implementation for the target-specific override, if a target overrides it. Such that you don't need the slightly confusing TargetIM indirection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given a line "LLVM-MCA-LATENCY: 100", the default Instrument could already provide you Desc and Data of "LLVM-MCA-LATENCY:" and "100" respectively. So I don't think you need the CustomInstrument here.
Ok, I can move the logic to base Instrument implementation
Such that you don't need the slightly confusing TargetIM indirection.
Without TargetIM generic customization will be disabled in case target-specific IM is used (since it will override createInstrument), unless all target IMs call base class createInstrument for unrecognized types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, supportsIntrumentType() is normally overridden in target-specific IMs, while we need to check for both common and target specific instruments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added target IM creation into default IM constructor to simplify the logic in the tool code.

Copy link
Member

@mshockwave mshockwave Aug 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless all target IMs call base class createInstrument for unrecognized types.

yes, thats what i meant when I said "call this base implementation for the target-specific override, if a target overrides it". Calling parent class's base implementation in an override function is a pretty common pattern in LLVM, and I would prefer that pattern to be used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll change the code to leave the logic in the base IM only without TargetIM indirection.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS Pushed the changes that remove targetIM

std::optional<unsigned> Latency;

public:
static const llvm::StringRef DESC_NAME;
explicit CustomInstrument(StringRef Data) : Instrument(DESC_NAME, Data) {
// Skip spaces and tabs.
unsigned Position = Data.find_first_not_of(" \t");
if (Position >= Data.size())
// We reached the end of the comment. Bail out.
return;
Data = Data.drop_front(Position);
auto [Name, Value] = Data.split(':');
if (Name.upper() == "LATENCY") {
Position = Value.find_first_not_of(" \t");
if (Position >= Value.size())
return;
auto Stripped = Value.drop_front(Position);
unsigned L = 0;
if (!Stripped.getAsInteger(10, L))
Latency = L;
}
}

bool canCustomize() const override { return bool(Latency); }

void customize(InstrDesc &ID) const override {
if (Latency) {
for (auto &W : ID.Writes)
W.Latency = *Latency;
ID.MaxLatency = *Latency;
}
}
};

const llvm::StringRef CustomInstrument::DESC_NAME = "CUSTOMIZE";

bool InstrumentManager::supportsInstrumentType(StringRef Type) const {
if (EnableDefaults && Type == CustomInstrument::DESC_NAME)
return true;
if (TargetIM)
return TargetIM->supportsInstrumentType(Type);
return false;
}

bool InstrumentManager::canCustomize(
const llvm::SmallVector<Instrument *> &IVec) const {
for (const auto I : IVec) {
if (I->canCustomize())
return true;
}
return false;
}

void InstrumentManager::customize(const llvm::SmallVector<Instrument *> &IVec,
InstrDesc &ID) const {
for (const auto I : IVec) {
if (I->canCustomize())
I->customize(ID);
}
}

UniqueInstrument InstrumentManager::createInstrument(llvm::StringRef Desc,
llvm::StringRef Data) {
if (!EnableDefaults)
return std::make_unique<Instrument>(Desc, Data);
if (Desc == CustomInstrument::DESC_NAME)
return std::make_unique<CustomInstrument>(Data);
if (TargetIM && TargetIM->supportsInstrumentType(Desc))
return TargetIM->createInstrument(Desc, Data);
return std::make_unique<Instrument>(Desc, Data);
}

SmallVector<UniqueInstrument>
InstrumentManager::createInstruments(const MCInst &Inst) {
if (TargetIM)
return TargetIM->createInstruments(Inst);
return SmallVector<UniqueInstrument>();
}

unsigned InstrumentManager::getSchedClassID(
const MCInstrInfo &MCII, const MCInst &MCI,
const llvm::SmallVector<Instrument *> &IVec) const {
if (TargetIM)
return TargetIM->getSchedClassID(MCII, MCI, IVec);
return MCII.get(MCI.getOpcode()).getSchedClass();
}

Expand Down
10 changes: 9 additions & 1 deletion llvm/lib/MCA/InstrBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,12 @@ InstrBuilder::createInstrDescImpl(const MCInst &MCI,
return std::move(Err);

// Now add the new descriptor.

if (IM.canCustomize(IVec)) {
IM.customize(IVec, *ID);
return *CustomDescriptors.emplace_back(std::move(ID));
}

bool IsVariadic = MCDesc.isVariadic();
if ((ID->IsRecyclable = !IsVariadic && !IsVariant)) {
auto DKey = std::make_pair(MCI.getOpcode(), SchedClassID);
Expand Down Expand Up @@ -676,7 +682,9 @@ STATISTIC(NumVariantInst, "Number of MCInsts that doesn't have static Desc");
Expected<std::unique_ptr<Instruction>>
InstrBuilder::createInstruction(const MCInst &MCI,
const SmallVector<Instrument *> &IVec) {
Expected<const InstrDesc &> DescOrErr = getOrCreateInstrDesc(MCI, IVec);
Expected<const InstrDesc &> DescOrErr = IM.canCustomize(IVec)
? createInstrDescImpl(MCI, IVec)
: getOrCreateInstrDesc(MCI, IVec);
if (!DescOrErr)
return DescOrErr.takeError();
const InstrDesc &D = *DescOrErr;
Expand Down
12 changes: 12 additions & 0 deletions llvm/test/tools/llvm-mca/X86/llvm-mca-markers-13.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=10 %s 2>&1 | FileCheck %s

# LLVM-MCA-CUSTOMIZE Latency:100
add (%eax), %eax
# LLVM-MCA-CUSTOMIZE
mov %eax, (%ebx)

# CHECK: Iterations: 10
# CHECK-NEXT: Instructions: 20
# CHECK-NEXT: Total Cycles: 1004
# CHECK-NEXT: Total uOps: 20
11 changes: 11 additions & 0 deletions llvm/test/tools/llvm-mca/X86/llvm-mca-markers-14.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 -iterations=10 %s 2>&1 | FileCheck %s

# LLVM-MCA-CUSTOMIZE Latency:100
add (%eax), %eax
mov %eax, (%ebx)

# CHECK: Iterations: 10
# CHECK-NEXT: Instructions: 20
# CHECK-NEXT: Total Cycles: 1103
# CHECK-NEXT: Total uOps: 20
11 changes: 7 additions & 4 deletions llvm/tools/llvm-mca/llvm-mca.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,15 @@ int main(int argc, char **argv) {

std::unique_ptr<mca::InstrumentManager> IM;
if (!DisableInstrumentManager) {
IM = std::unique_ptr<mca::InstrumentManager>(
TheTarget->createInstrumentManager(*STI, *MCII));
std::unique_ptr<mca::InstrumentManager> TargetIM =
std::unique_ptr<mca::InstrumentManager>(
TheTarget->createInstrumentManager(*STI, *MCII));
IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII, true,
std::move(TargetIM));
}
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).
// If -disable-cb flag is set then we use the base class with default
// behavior (which does nothing).
IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
}

Expand Down
21 changes: 15 additions & 6 deletions llvm/unittests/tools/llvm-mca/MCATestBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,25 @@ void MCATestBase::SetUp() {
ASSERT_TRUE(IP);
}

Error MCATestBase::runBaselineMCA(json::Object &Result, ArrayRef<MCInst> Insts,
ArrayRef<mca::View *> Views,
const mca::PipelineOptions *PO) {
Error MCATestBase::runBaselineMCA(
json::Object &Result, ArrayRef<MCInst> Insts,
ArrayRef<mca::View *> Views,
const mca::PipelineOptions *PO,
SmallVector<std::pair<StringRef, StringRef>> Descs) {
mca::Context MCA(*MRI, *STI);

// Default InstrumentManager
auto IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
// Enable instruments when descriptions are provided
auto IM =
std::make_unique<mca::InstrumentManager>(*STI, *MCII, !Descs.empty());
mca::InstrBuilder IB(*STI, *MCII, *MRI, MCIA.get(), *IM, /*CallLatency=*/100);

const SmallVector<mca::Instrument *> Instruments;
SmallVector<mca::Instrument *> Instruments;
SmallVector<mca::UniqueInstrument> InstrumentsOwner;
for (const auto &Desc : Descs) {
auto I = IM->createInstrument(Desc.first, Desc.second);
Instruments.push_back(I.get());
InstrumentsOwner.push_back(std::move(I));
}
SmallVector<std::unique_ptr<mca::Instruction>> LoweredInsts;
for (const auto &MCI : Insts) {
Expected<std::unique_ptr<mca::Instruction>> Inst =
Expand Down
9 changes: 6 additions & 3 deletions llvm/unittests/tools/llvm-mca/MCATestBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "llvm/MC/MCTargetOptions.h"
#include "llvm/MC/TargetRegistry.h"
#include "llvm/MCA/Context.h"
#include "llvm/MCA/InstrBuilder.h"
#include "llvm/TargetParser/SubtargetFeature.h"
#include "llvm/TargetParser/Triple.h"

Expand Down Expand Up @@ -73,9 +74,11 @@ class MCATestBase : public ::testing::Test {
/// Utility function to run MCA with (nearly) the same configuration as the
/// `llvm-mca` tool to verify result correctness.
/// This function only displays on SummaryView by default.
virtual Error runBaselineMCA(json::Object &Result, ArrayRef<MCInst> Insts,
ArrayRef<mca::View *> Views = {},
const mca::PipelineOptions *PO = nullptr);
virtual Error
runBaselineMCA(json::Object &Result, ArrayRef<MCInst> Insts,
ArrayRef<mca::View *> Views = {},
const mca::PipelineOptions *PO = nullptr,
SmallVector<std::pair<StringRef, StringRef>> Descs = {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use ArrayRef here: https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h (see the "Note")

};

} // end namespace mca
Expand Down
23 changes: 23 additions & 0 deletions llvm/unittests/tools/llvm-mca/X86/TestIncrementalMCA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,26 @@ TEST_F(X86TestBase, TestVariantInstructionsSameAddress) {
Expected<unsigned> Cycles = P->run();
ASSERT_TRUE(static_cast<bool>(Cycles));
}

TEST_F(X86TestBase, TestInstructionCustomization) {
const unsigned ExplicitLatency = 100;
SmallVector<MCInst> MCIs;
MCInst InstructionToAdd = MCInstBuilder(X86::XOR64rr)
.addReg(X86::RAX)
.addReg(X86::RAX)
.addReg(X86::RAX);
MCIs.push_back(InstructionToAdd);
SmallVector<std::pair<StringRef, StringRef>> InstrDescs;
InstrDescs.push_back(
std::make_pair(StringRef("CUSTOMIZE"), StringRef("Latency:100")));

// Run the baseline.
json::Object BaselineResult;
auto E = runBaselineMCA(BaselineResult, MCIs, {}, nullptr, InstrDescs);
ASSERT_FALSE(bool(E)) << "Failed to run baseline";
auto *BaselineObj = BaselineResult.getObject("SummaryView");
auto V = BaselineObj->getInteger("TotalCycles");
ASSERT_TRUE(V);
// Additional 3 cycles for Dispatch, Executed and Retired states
ASSERT_EQ(unsigned(*V), ExplicitLatency + 3) << "Total cycles do not match";
}
Loading