Page MenuHomePhabricator

[test-suite] Added Image Processing Kernels Using Benchmark Library: Blur Algorithms
ClosedPublic

Authored by proton on Jul 14 2018, 12:42 AM.

Details

Summary

Added mean filter blur and gaussian blur using benchmark library

Apply D49339 before applying this as D49339 contains some helper functions used here.

Diff Detail

Repository
rL LLVM

Event Timeline

proton created this revision.Jul 14 2018, 12:42 AM
dberris added inline comments.Jul 15 2018, 5:29 PM
MicroBenchmarks/ImageProcessing/Blur/blur.h
15–18 ↗(On Diff #155554)

Do these functions need the array sizes at all? Why not just use int **?

MicroBenchmarks/ImageProcessing/Blur/main.cpp
167 ↗(On Diff #155554)

You probably want a ::benchmark::DoNotOptimize(...) around this and other calls to the kernel.

proton updated this revision to Diff 155760.Jul 16 2018, 2:10 PM

Changed function parameters from "int image[HEIGHT][WIDTH]" to "int *image"

proton added inline comments.Jul 16 2018, 2:16 PM
MicroBenchmarks/ImageProcessing/Blur/main.cpp
167 ↗(On Diff #155554)

I don't think I can use "::benchmark::DoNotOptimize(...)" here as the functions return void and DoNotOptimize forces the compiler to flush pending writes to memory.

Meinersbur added inline comments.Jul 31 2018, 6:23 PM
MicroBenchmarks/ImageProcessing/Blur/main.cpp
156–157 ↗(On Diff #155760)

[nit] empty lines

175–177 ↗(On Diff #155760)

@dberris Would ::benchmark::ClobberMemory() be a viable alternative?

dberris added inline comments.Jul 31 2018, 6:44 PM
MicroBenchmarks/ImageProcessing/Blur/main.cpp
175–177 ↗(On Diff #155760)

It may be.

Meinersbur added inline comments.Aug 3 2018, 11:31 AM
MicroBenchmarks/ImageProcessing/Blur/boxBlurKernel.cpp
10–11 ↗(On Diff #155760)

Can you change inpImage/outImage to use C99 array parameter syntax?

MicroBenchmarks/ImageProcessing/Blur/main.cpp
175–177 ↗(On Diff #155760)

I suggest to leave it as is for the moment. If we found a canonical approach, we can change this and the other benchmarks afterwards.

Could you add a comment that this is supposed keep the compiler to optimize the computation away?

proton updated this revision to Diff 159183.Aug 4 2018, 7:36 AM
Meinersbur accepted this revision.Aug 6 2018, 8:32 AM

LGTM

MicroBenchmarks/ImageProcessing/Blur/main.cpp
175–177 ↗(On Diff #155760)

@proton Please don't forget to add a comment about that this code is meant to inhibit too-aggressive compiler optimizations.

This revision is now accepted and ready to land.Aug 6 2018, 8:32 AM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.