Skip to content

[llvm-exegesis] Simplify validation event string conversion functions #82092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[llvm-exegesis] Simplify validation event string conversion functions #82092

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 29 additions & 32 deletions llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,24 @@ static constexpr const char kIntegerPrefix[] = "i_0x";
static constexpr const char kDoublePrefix[] = "f_";
static constexpr const char kInvalidOperand[] = "INVALID";

// When adding a new validation counter, a new event type needs to be added
// to llvm::exegesis::ValidationEvent, a mapping needs to be added below,
// a command line option needs to be added in llvm-exegesis.cpp, and the
// event type needs to be added (with matching naming) in TargetPfmCounters.td
// and appropriate mappings need to be added in the relevant architecture
// Pfm descriptions.
static constexpr const std::pair<llvm::exegesis::ValidationEvent,
llvm::StringRef>
kValidationEventNames[] = {
{llvm::exegesis::InstructionRetired, "instructions-retired"},
Copy link
Contributor

Choose a reason for hiding this comment

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

What about putting the name (e.g. "instructions-retired") in the tabledef ValidationEvent class ? This would avoid any kind of synchronization to have to be done manually.

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 looked into doing that, but the way the Exegesis TableGen is setup, everything that gets TableGened ends up in a target-specific ExegesisTarget implementation. Moving it there naively would mean duplication across the targets and move this code somewhere where it makes less sense.

We could probably make some TableGen/build system changes to get a target-independent .inc with this information, but that seemed excessive to me, unless I'm overthinking it and it's a lot simpler than that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I'm not a big fan of having validation counter tables spread all over the place (enum, td files, flags, and now this table). Let me think it over.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a proposal in #82256

{llvm::exegesis::L1DCacheLoadMiss, "l1d-cache-load-misses"},
{llvm::exegesis::L1DCacheStoreMiss, "l1d-cache-store-misses"},
{llvm::exegesis::L1ICacheLoadMiss, "l1i-cache-load-misses"},
{llvm::exegesis::DataTLBLoadMiss, "data-tlb-load-misses"},
{llvm::exegesis::DataTLBStoreMiss, "data-tlb-store-misses"},
{llvm::exegesis::InstructionTLBLoadMiss,
"instruction-tlb-load-misses"}};

namespace llvm {

namespace {
Expand Down Expand Up @@ -194,43 +212,22 @@ template <> struct SequenceElementTraits<exegesis::BenchmarkMeasure> {
};

const char *validationEventToString(exegesis::ValidationEvent VE) {
switch (VE) {
case exegesis::ValidationEvent::InstructionRetired:
return "instructions-retired";
case exegesis::ValidationEvent::L1DCacheLoadMiss:
return "l1d-cache-load-misses";
case exegesis::ValidationEvent::L1DCacheStoreMiss:
return "l1d-cache-store-misses";
case exegesis::ValidationEvent::L1ICacheLoadMiss:
return "l1i-cache-load-misses";
case exegesis::ValidationEvent::DataTLBLoadMiss:
return "data-tlb-load-misses";
case exegesis::ValidationEvent::DataTLBStoreMiss:
return "data-tlb-store-misses";
case exegesis::ValidationEvent::InstructionTLBLoadMiss:
return "instruction-tlb-load-misses";
for (const auto [ValidationEventType, ValidationEventName] :
kValidationEventNames) {
if (VE == ValidationEventType)
return ValidationEventName.data();
}
llvm_unreachable("Unhandled exegesis::ValidationEvent enum");
}

Expected<exegesis::ValidationEvent> stringToValidationEvent(StringRef Input) {
if (Input == "instructions-retired")
return exegesis::ValidationEvent::InstructionRetired;
else if (Input == "l1d-cache-load-misses")
return exegesis::ValidationEvent::L1DCacheLoadMiss;
else if (Input == "l1d-cache-store-misses")
return exegesis::ValidationEvent::L1DCacheStoreMiss;
else if (Input == "l1i-cache-load-misses")
return exegesis::ValidationEvent::L1ICacheLoadMiss;
else if (Input == "data-tlb-load-misses")
return exegesis::ValidationEvent::DataTLBLoadMiss;
else if (Input == "data-tlb-store-misses")
return exegesis::ValidationEvent::DataTLBStoreMiss;
else if (Input == "instruction-tlb-load-misses")
return exegesis::ValidationEvent::InstructionTLBLoadMiss;
else
return make_error<StringError>("Invalid validation event string",
errc::invalid_argument);
for (const auto [ValidationEventType, ValidationEventName] :
kValidationEventNames) {
if (Input == ValidationEventName)
return ValidationEventType;
}
return make_error<StringError>("Invalid validation event string",
errc::invalid_argument);
}

template <>
Expand Down