[PATCH] D46735: [Test-Suite] Added Box Blur And Sobel Edge Detection

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

Are you writing these from scratch? If so, I'd like to make some suggestions:

- Please aim for a runtime for 0.5-1 second on typical hardware. Shorter benchmarks tend to be hard to time correctly, running longer doesn't increase precision in our experience.
- I'd go for "MultiSource" benchmarks, even if they end up being a single sourcefile; Producing multiple executables out of a single directory of source files is more complicated than it seems.
- Did you check the amount of time spent on initializing and printing the image compared to the main operation of the benchmark?



================
Comment at: MultiSource/Benchmarks/7zip/CMakeLists.txt:4
 list(APPEND CFLAGS -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread)
-list(APPEND CXXFLAGS -Wno-error=c++11-narrowing -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread)
+list(APPEND CXXFLAGS -Wno-error=narrowing -DBREAK_HANDLER -DUNICODE -D_UNICODE -I${CMAKE_CURRENT_SOURCE_DIR}/C -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/myWindows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP/include_windows -I${CMAKE_CURRENT_SOURCE_DIR}/CPP -I. -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DNDEBUG -D_REENTRANT -DENV_UNIX -D_7ZIP_LARGE_PAGES -pthread)
 list(APPEND LDFLAGS -lstdc++ -pthread)
----------------
Unrelated, please keep it separate.


================
Comment at: SingleSource/Benchmarks/ImageProcessing/blur/blur.cpp:27
+int main(int argc, char *argv[])
+{
+    int ** image = (int**)malloc(HEIGHT*sizeof(int *));
----------------
If you develop the benchmark from scratch it would be nice if you could design them in a way that they receive a single number as argument in a way that increasing the number makes the benchmark run longer; it's fine to print different results for different numbers.

This allows to choose a good size for different devices in the future.


================
Comment at: SingleSource/Benchmarks/ImageProcessing/sobel/Makefile:6
+
+HASH_PROGRAM_OUTPUT = 1
+
----------------
In my experience benchmarks using `HASH_PROGRAM_OUTPUT` are very noisy (at least on the systems I care about), because we inadvertantly end up testing how fast the kernel can pipe output from a process into the next (which is the md5 utility).
Rather go for a simple checksumming mechanism as part of the sourcecode.


Repository:
  rT test-suite

https://reviews.llvm.org/D46735