Page MenuHomePhabricator

[llvm-exegesis][NFC] Disassociate snippet generators from benchmark runners
ClosedPublic

Authored by mstojanovic on Jan 17 2020, 8:09 AM.

Details

Summary

The addition of inverse_throughput mode highlighted the disjointedness of snippet generators and benchmark runners because it used the UopsSnippetGenerator with the LatencyBenchmarkRunner. To keep the code consistent tie the snippet generators to parallelization/serialization rather than their benchmark runners.

Renaming LatencySnippetGenerator -> SerialSnippetGenerator.
Renaming UopsSnippetGenerator -> ParallelSnippetGenerator.
Renaming Uops -> Parallel in types and functions related to the ParallelSnippetGenerator.

Diff Detail

Event Timeline

mstojanovic created this revision.Jan 17 2020, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2020, 8:09 AM
mstojanovic marked 2 inline comments as done.Jan 17 2020, 8:15 AM
mstojanovic added inline comments.
llvm/tools/llvm-exegesis/lib/ParallelSnippetGenerator.cpp
17–20

I'm not sure how this can be reworded to reflect that it deals with parallel instructions in general rather than just the uops case.

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp
11–19

It's not easy to determine which headers are needed here when splitting the files. Most of them seem to be unused.

courbet added inline comments.Jan 17 2020, 8:38 AM
llvm/tools/llvm-exegesis/lib/ParallelSnippetGenerator.cpp
17–20

Maybe :
"Ideally we would like the only limitation on executing instructions to be the availability of the CPU resources (e.g. execution ports) needed to execute them, instead of the availability of their data dependencies."

Updated the comment.

courbet accepted this revision.Jan 20 2020, 12:25 AM

Thanks.

llvm/tools/llvm-exegesis/lib/SerialSnippetGenerator.cpp
11–19

I could not find anything obvious that would require any of these and not already use din the header.

Headers are missing for vector, iota and shuffle though.

This revision is now accepted and ready to land.Jan 20 2020, 12:25 AM
mstojanovic marked an inline comment as done.
mstojanovic edited the summary of this revision. (Show Details)

Added includes for vector, iota and shuffle.
Renamed writeUopsSnippetHtml() -> writeParallelSnippetHtml().

llvm/tools/llvm-exegesis/lib/ParallelSnippetGenerator.cpp
42

Should this be reworded similarly to the comment above?

courbet added inline comments.Jan 20 2020, 7:04 AM
llvm/tools/llvm-exegesis/lib/ParallelSnippetGenerator.cpp
42

I think this one is fine since we gave ports as an example for resources before.

Ok, thank you for the review!

This revision was automatically updated to reflect the committed changes.