[PATCH] D49503: [test-suite] Add Image Processing Kernels Using Benchmark Library: Dither Algorithms

[Pankaj via Phabricator via llvm-commits]

proton added inline comments.


================
Comment at: test-suite/trunk/MicroBenchmarks/ImageProcessing/Dither/main.cpp:144-154
+#if MINIMUM_DIM > 128
+BENCHMARK(BENCHMARK_FLOYD_DITHER)
+    ->RangeMultiplier(2)
+    ->Range(128, MINIMUM_DIM)
+    ->Unit(benchmark::kMicrosecond);
+#else
+BENCHMARK(BENCHMARK_FLOYD_DITHER)
----------------
Meinersbur wrote:
> MatzeB wrote:
> > I just noticed an odd effect here. A benchmark run now gives us 12 results for the same function running.
> > 
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/512/3
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/512/2
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/256/3
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/256/2
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/128/2
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/128/3
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/256/4
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/512/4
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/128/4
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/512/8
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/128/8
> > test-suite :: MicroBenchmarks/ImageProcessing/Dither/Dither.test:BENCHMARK_ORDERED_DITHER/256/8
> > 
> > I just had a change that was mostly improving things but happened to slightly regress this benchmark. Unfortunately this effect is now multiplied by 12 so this benchmark has a far bigger weight than others...
> Do you suggest to reduce the number of variants?
We can reduce the number of runs here but I think instead of hardcoding it here (like it is now), we can pass it as an argument from cmake file. 
Earlier I dropped this idea because I remember someone mentioning that multiple test size is not used in the test suite now so it seemed unnecessary then. 
This can help someone who may want to see the effect of some optimization (say tiling) on different input Matrix sizes and different zoom ratio here.
For hardcoding it, we can remove the multiple zoom ratio here 
from
    b->Args({i, 2});
    b->Args({i, 3});
    b->Args({i, 4});
    b->Args({i, 8});
to 
    b->Args({i, 2});
    b->Args({i, 8});
See Line: 107


Repository:
  rL LLVM

https://reviews.llvm.org/D49503