Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 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
47 changes: 42 additions & 5 deletions llvm/include/llvm/MCA/CustomBehaviour.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,41 @@ class Instrument {
/// The instrumentation data
const StringRef Data;

std::optional<unsigned> Latency;

public:
Instrument(StringRef Desc, StringRef Data) : Desc(Desc), Data(Data) {}
Instrument(StringRef Desc, StringRef Data) : Desc(Desc), Data(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);
Copy link
Member

Choose a reason for hiding this comment

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

can we use StringRef::trim to replace this part of the code?

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;
}
}

Instrument() : Instrument("", "") {}

virtual ~Instrument() = default;

virtual bool canCustomize() const { return bool(Latency); }
virtual void customize(InstrDesc &ID) const {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not adding this virtual function here and implement the customization logics directly in InstrumentManager::customize instead.
With such approach, we can also avoid storing Latency in this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latency must be stored in the instrument since it may be (and normally is) different in different instrumentation regions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ping - any additional comments?

if (Latency) {
for (auto &W : ID.Writes)
W.Latency = *Latency;
Copy link
Member

Choose a reason for hiding this comment

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

@adibiagio pointed this out in our offline discussions: an instruction may perform multiple writes, is there any way we could designate which write to apply this latency customization? It doesn't have to be in this patch, but a TODO comment would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, fine grain customization makes sense. Added a comment

ID.MaxLatency = *Latency;
}
}

StringRef getDesc() const { return Desc; }
StringRef getData() const { return Data; }
};
Expand All @@ -143,19 +171,21 @@ class LLVM_ABI InstrumentManager {
protected:
const MCSubtargetInfo &STI;
const MCInstrInfo &MCII;
bool EnableInstruments;

public:
InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII)
: STI(STI), MCII(MCII) {}
InstrumentManager(const MCSubtargetInfo &STI, const MCInstrInfo &MCII,
bool EnableInstruments = false)
: STI(STI), MCII(MCII), EnableInstruments(EnableInstruments) {};

virtual ~InstrumentManager() = default;

/// Returns true if llvm-mca should ignore instruments.
virtual bool shouldIgnoreInstruments() const { return true; }
virtual bool shouldIgnoreInstruments() const { return !EnableInstruments; }

// 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 +205,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
24 changes: 24 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,6 +43,29 @@ CustomBehaviour::getEndViews(llvm::MCInstPrinter &IP,
return std::vector<std::unique_ptr<View>>();
}

static const llvm::StringRef CustomInstrumentName = "CUSTOMIZE";

bool InstrumentManager::supportsInstrumentType(StringRef Type) const {
return EnableInstruments && Type == CustomInstrumentName;
}

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) {
return std::make_unique<Instrument>(Desc, Data);
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
12 changes: 8 additions & 4 deletions llvm/tools/llvm-mca/llvm-mca.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,10 +513,14 @@ int main(int argc, char **argv) {
if (!DisableInstrumentManager) {
IM = std::unique_ptr<mca::InstrumentManager>(
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).
if (!IM) {
// If the target doesn't have its own IM implemented we use base class
// with instruments enabled.
IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII, true);
}
} else {
// If the -disable-cb flag is set then we use the default base class
// implementation (which does nothing).
IM = std::make_unique<mca::InstrumentManager>(*STI, *MCII);
}

Expand Down
20 changes: 14 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,24 @@ 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";
}