This is an archive of the discontinued LLVM Phabricator instance.

[llvm-exegesis] Allow benchmarking arbitrary code snippets.
ClosedPublic

Authored by courbet on Sep 13 2018, 8:30 AM.

Details

Summary

This is a step towards fixing PR38048.

Note that right now the measurements are given per instruction. We'll
need to give measurements a per code snippet and update the analysis (PR38731).

Diff Detail

Repository
rL LLVM

Event Timeline

courbet created this revision.Sep 13 2018, 8:30 AM
courbet retitled this revision from [llvm-exegesis][NFC] Remove dead parameter. to [llvm-exegesis] Allow benchmarking arbitrary code snippets..Sep 13 2018, 8:32 AM
courbet edited the summary of this revision. (Show Details)
courbet updated this revision to Diff 165304.Sep 13 2018, 8:33 AM

Change diff title.

gchatelet added inline comments.Sep 18 2018, 2:09 AM
tools/llvm-exegesis/lib/BenchmarkRunner.cpp
39 ↗(On Diff #165304)
while(Code.size() < (BC.size() + MinInstruction))
tools/llvm-exegesis/llvm-exegesis.cpp
108 ↗(On Diff #165304)

Can we make this more explicit?

const size_t CountSetFlags =
 (OpcodeName.empty() ? 0 : 1) +
 (OpcodeIndex == 0 ? 0 : 1) +
 (SnippetsFile.empty() ? 0 : 1);
if(CountSetFlags != 1)
   ...
159 ↗(On Diff #165304)

newline before

161 ↗(On Diff #165304)

A streamer -> An MCStreamer

162 ↗(On Diff #165304)

which -> with

213 ↗(On Diff #165304)

AFAIK llvm does not provide any tool to do it. Not sure if it's worth the addition though.

231 ↗(On Diff #165304)

std::string Filename -> llvm::StringRef Filename ?

236 ↗(On Diff #165304)

Why pass the filename/message in the BenchmarkFailure and remove the llvm::errs line?

254 ↗(On Diff #165304)

Add a comment explaining what 0 is in this context.

courbet updated this revision to Diff 165978.Sep 18 2018, 7:49 AM
courbet marked 9 inline comments as done.

Address comments.

Harbormaster completed remote builds in B22800: Diff 165979.
courbet updated this revision to Diff 166259.Sep 20 2018, 4:19 AM

update doc

gchatelet added inline comments.Sep 24 2018, 5:40 AM
tools/llvm-exegesis/llvm-exegesis.cpp
192 ↗(On Diff #166291)

Add comments (or variables) to explain the arguments

194 ↗(On Diff #166291)

Maybe we want to crash here?

198 ↗(On Diff #166291)

explain Value.size() * 4

254 ↗(On Diff #165304)

Not done.

courbet updated this revision to Diff 166687.Sep 24 2018, 7:47 AM
courbet marked 2 inline comments as done.

Address comments.

courbet marked an inline comment as done.Sep 24 2018, 7:49 AM
courbet updated this revision to Diff 166689.Sep 24 2018, 7:54 AM

Add more comments.

courbet updated this revision to Diff 166692.Sep 24 2018, 7:57 AM

rely on implicit default asm dialect

gchatelet accepted this revision.Sep 24 2018, 8:10 AM
This revision is now accepted and ready to land.Sep 24 2018, 8:10 AM
courbet updated this revision to Diff 166820.Sep 25 2018, 12:32 AM

add missing stack setup for EFLAGS.

This revision was automatically updated to reflect the committed changes.