Merge preview/rvv-exegesis with main (dirty change history) #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e1turin
commented
Feb 7, 2025
Comment on lines
+80
to
+84
| // MERGEME: useful operator? | ||
| //bool operator==(const BenchmarkKey &RHS) const { | ||
| // return Config == RHS.Config && | ||
| // Instructions[0].getOpcode() == RHS.Instructions[0].getOpcode(); | ||
| //} |
Comment on lines
+150
to
+160
| bool DryRun = DryRunMeasurement; | ||
| auto PS = ET.withSavedState(); | ||
| CrashRecoveryContext CRC; | ||
| CrashRecoveryContext::Enable(); | ||
| const bool Crashed = !CRC.RunSafely([this, Counter, ScratchPtr]() { | ||
| Counter->start(); | ||
| this->Function(ScratchPtr); | ||
| Counter->stop(); | ||
| }); | ||
| const bool Crashed = | ||
| !CRC.RunSafely([this, Counter, ScratchPtr, DryRun]() { | ||
| Counter->start(); | ||
| if (!DryRun) | ||
| this->Function(ScratchPtr); | ||
| Counter->stop(); | ||
| }); |
There was a problem hiding this comment.
Does DryRunMeasurement supported?
Comment on lines
+57
to
+59
| Benchmark BenchmarkResult; | ||
| object::OwningBinary<object::ObjectFile> ObjectFile; | ||
|
|
There was a problem hiding this comment.
revert. It should remain private
Comment on lines
-148
to
-158
| #else | ||
| void ConfiguredEvent::initRealEvent(pid_t ProcessID, const int GroupFD) {} | ||
|
|
||
| Expected<SmallVector<int64_t>> | ||
| ConfiguredEvent::readOrError(StringRef /*unused*/) const { | ||
| return make_error<StringError>("Not implemented", | ||
| errc::function_not_supported); | ||
| } | ||
|
|
||
| ConfiguredEvent::~ConfiguredEvent() = default; | ||
| #endif // HAVE_LIBPFM |
There was a problem hiding this comment.
Do we really require LIBPFM?
Comment on lines
-218
to
-243
| #else | ||
|
|
||
| void CounterGroup::initRealEvent(pid_t ProcessID) {} | ||
|
|
||
| void CounterGroup::start() {} | ||
|
|
||
| void CounterGroup::stop() {} | ||
|
|
||
| Expected<SmallVector<int64_t, 4>> | ||
| CounterGroup::readOrError(StringRef /*unused*/) const { | ||
| if (IsDummyEvent) { | ||
| SmallVector<int64_t, 4> Result; | ||
| Result.push_back(42); | ||
| return Result; | ||
| } | ||
| return make_error<StringError>("Not implemented", errc::io_error); | ||
| } | ||
|
|
||
| Expected<SmallVector<int64_t>> | ||
| CounterGroup::readValidationCountersOrError() const { | ||
| return SmallVector<int64_t>(0); | ||
| } | ||
|
|
||
| int CounterGroup::numValues() const { return 1; } | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Do we really require LIBPFM?
Comment on lines
-53
to
-56
| static cl::opt<int> OpcodeIndex( | ||
| "opcode-index", | ||
| cl::desc("opcode to measure, by index, or -1 to measure all opcodes"), | ||
| cl::cat(BenchmarkOptions), cl::init(0)); |
There was a problem hiding this comment.
revert. Use modern option instead same OpcodeIndices
Comment on lines
+129
to
+133
| static cl::opt<std::string> | ||
| InputFile(cl::Positional, | ||
| cl::desc("Input benchmarks file to resume or snippet file"), | ||
| cl::init("-"), cl::cat(Options)); | ||
|
|
There was a problem hiding this comment.
revert as there is modern benchmarks-file option
Comment on lines
-115
to
+204
| static cl::opt<BenchmarkPhaseSelectorE> BenchmarkPhaseSelector( | ||
| "benchmark-phase", | ||
| cl::desc( | ||
| "it is possible to stop the benchmarking process after some phase"), | ||
| cl::cat(BenchmarkOptions), | ||
| cl::values( | ||
| clEnumValN(BenchmarkPhaseSelectorE::PrepareSnippet, "prepare-snippet", | ||
| "Only generate the minimal instruction sequence"), | ||
| clEnumValN(BenchmarkPhaseSelectorE::PrepareAndAssembleSnippet, | ||
| "prepare-and-assemble-snippet", | ||
| "Same as prepare-snippet, but also dumps an excerpt of the " | ||
| "sequence (hex encoded)"), | ||
| clEnumValN(BenchmarkPhaseSelectorE::AssembleMeasuredCode, | ||
| "assemble-measured-code", | ||
| "Same as prepare-and-assemble-snippet, but also creates the " | ||
| "full sequence " | ||
| "that can be dumped to a file using --dump-object-to-disk"), | ||
| clEnumValN( | ||
| BenchmarkPhaseSelectorE::Measure, "measure", | ||
| "Same as prepare-measured-code, but also runs the measurement " | ||
| "(default)")), | ||
| cl::init(BenchmarkPhaseSelectorE::Measure)); | ||
| static const auto BenchmarkPhasesOptValues = cl::values( | ||
| clEnumValN(BenchmarkPhaseSelectorE::PrepareSnippet, "prepare-snippet", | ||
| "Only generate the minimal instruction sequence"), | ||
| clEnumValN(BenchmarkPhaseSelectorE::PrepareAndAssembleSnippet, | ||
| "prepare-and-assemble-snippet", | ||
| "Same as prepare-snippet, but also dumps an excerpt of the " | ||
| "sequence (hex encoded)"), | ||
| clEnumValN(BenchmarkPhaseSelectorE::AssembleMeasuredCode, | ||
| "assemble-measured-code", | ||
| "Same as prepare-and-assemble-snippet, but also creates the " | ||
| "full sequence " | ||
| "that can be dumped to a file using --dump-object-to-disk"), | ||
| clEnumValN(BenchmarkPhaseSelectorE::Measure, "measure", | ||
| "Same as prepare-measured-code, but also runs the measurement " | ||
| "(default)")); | ||
|
|
||
| static cl::opt<BenchmarkPhaseSelectorE> | ||
| StopAfter("stop-after-phase", | ||
| cl::desc("Stop the benchmarking process after some phase"), | ||
| cl::cat(BenchmarkOptions), BenchmarkPhasesOptValues, | ||
| cl::init(BenchmarkPhaseSelectorE::Measure)); | ||
|
|
||
| static cl::alias BenchmarkPhaseSelector("benchmark-phase", | ||
| cl::desc("Alias of -stop-after-phase"), | ||
| cl::aliasopt(StopAfter)); | ||
|
|
||
| static cl::opt<BenchmarkPhaseSelectorE> StartBefore( | ||
| "start-before-phase", | ||
| cl::desc("Resume the benchmarking process before a certain phase"), | ||
| cl::cat(BenchmarkOptions), BenchmarkPhasesOptValues, | ||
| cl::init(BenchmarkPhaseSelectorE::PrepareSnippet)); |
There was a problem hiding this comment.
Carefully resolve diff: refactored options structure in modern version
Comment on lines
+693
to
+695
| // MERGEME: eliminated code in main. | ||
| //std::vector<BenchmarkRunner::RunnableConfiguration> RunnableConfigs; | ||
| //SmallVector<unsigned> Repetitions; |
There was a problem hiding this comment.
MERGEME: does exegesis support repetitions? RunnableConfigs looks like a legacy
Comment on lines
-479
to
+596
| if (BenchmarkPhaseSelector == BenchmarkPhaseSelectorE::Measure && | ||
| !UseDummyPerfCounters) { | ||
| #ifndef HAVE_LIBPFM | ||
| ExitWithError( | ||
| "benchmarking unavailable, LLVM was built without libpfm. You can " | ||
| "pass --benchmark-phase=... to skip the actual benchmarking or " | ||
| "--use-dummy-perf-counters to not query the kernel for real event " | ||
| "counts."); | ||
| #else | ||
| if (StopAfter == BenchmarkPhaseSelectorE::Measure && !UseDummyPerfCounters) { | ||
| #ifdef HAVE_LIBPFM |
e1turin
changed the title
e1turin
force-pushed
the
merge/rvv-exegesis-full
branch
from
40aa183 to
147aba3
Compare
e1turin
force-pushed
the
merge/rvv-exegesis-full
branch
from
b648ecd to
16168af
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nasty process of merging 2 commits (llvm#114149) onto main (dirty branch yet). With a purpose to control made changes, there are
merge.diffin the root and log of changed files instatus_merge.txt.// MERGEME:comments when it should recheck changes.llvm/lib/RISCV/Target.cppcontains tidy merge conflicts to be solved in future commits.Implementation details