Page MenuHomePhabricator

[MicroBenchmark] Add initial LoopInterchange test/benchmark.
ClosedPublic

Authored by fhahn on Oct 9 2018, 10:35 AM.

Details

Summary

This patch adds a first test case specifically for LoopInterchange. I am
not sure if MicroBenchmarks is the best place for this, but it seems
like a good way to benchmark/tests a set of loops targeted at loop
interchange.

Diff Detail

Repository
rL LLVM

Event Timeline

fhahn created this revision.Oct 9 2018, 10:35 AM
MatzeB added a subscriber: MatzeB.Oct 16 2018, 4:15 PM

Did you make sure this typically finished in 0.5-1s of time?

MicroBenchmarks/LoopInterchange/main.cpp
64 ↗(On Diff #168830)

Does this need to be enclosed in #ifdef BENCHMARK_LIB as well? Or alternatively could you just drop all the #ifdefs to simplify the code?

MatzeB added inline comments.Oct 16 2018, 4:17 PM
MicroBenchmarks/LoopInterchange/main.cpp
25 ↗(On Diff #168830)

I think y may overflow here and trigger UB, you should probably use an unsigned variable for it.

39 ↗(On Diff #168830)

Does this need the ./ in front of the path? I would also recommend to abort if the output file cannot be written instead of continuing silently.

MatzeB added inline comments.Oct 16 2018, 4:19 PM
MicroBenchmarks/LoopInterchange/main.cpp
19 ↗(On Diff #168830)

M and N are unused?

58–59 ↗(On Diff #168830)

Is the printf here a good idea? You'll artificially make one iteration slower which could possibly make the benchmark library take longer to stabilize until the result is statistically significant.

fhahn updated this revision to Diff 169932.Oct 16 2018, 6:09 PM
fhahn marked 2 inline comments as done.

Thanks. I've dropped the range. On a cortex-a72, it takes around 1 second.

MicroBenchmarks/LoopInterchange/main.cpp
19 ↗(On Diff #168830)

Fixed, looks like I uploaded an old version of the diff.

58–59 ↗(On Diff #168830)

I've added a comment to the code. It's never called with state.range(0), so the printf should be never executed, but ensure the test1 calls cannot be optimized away.

64 ↗(On Diff #168830)

I've removed them all.

homerdin added inline comments.Oct 17 2018, 7:16 AM
MicroBenchmarks/LoopInterchange/main.cpp
58–59 ↗(On Diff #168830)

Can preventing the call being optimized away be accomplished using benchmark::DoNotOptimize() or benchmark::ClobberMemory()?

MatzeB accepted this revision.Oct 22 2018, 12:54 PM

Please try whether using DoNotOptimize as suggested works. With that addressed LGTM.

This revision is now accepted and ready to land.Oct 22 2018, 12:54 PM
fhahn added a comment.Oct 23 2018, 3:38 AM

Thanks! I'll submit it in a few days, with DoNotOptimize.

MicroBenchmarks/LoopInterchange/main.cpp
58–59 ↗(On Diff #168830)

Thanks for pointing me to that function, it looks like it does the same thing in a much clearer way!

fhahn updated this revision to Diff 175643.Nov 28 2018, 2:47 AM
fhahn marked an inline comment as done.

Use DoNotOptimize

This revision was automatically updated to reflect the committed changes.