[PATCH] D49246: Added Image Processing Kernels Using Benchmark Library

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 12 15:27:58 PDT 2018


Meinersbur added a comment.

@proton Thank you for the new benchmarks!

IHMO we should have one review and commit per benchmark. This would make it easier to revert an individual benchmark when it fails.



================
Comment at: MicroBenchmarks/ImageProcessing/Blur/main.cpp:130
+
+// We read entire Image, but only convert a part of image to blur to see how it
+// Scales
----------------
[typo] We read _the_ entire


================
Comment at: MicroBenchmarks/ImageProcessing/utils/ImageHelper.cpp:8
+    for (int j = 0; j < width; j++) {
+      image[i * width + j] = rand() % 256;
+    }
----------------
Other benchmarks use `drand48()`. Maybe we should use it as well.


================
Comment at: MicroBenchmarks/ImageProcessing/utils/ImageHelper.h:5-10
+#include <iostream>
+#include <fstream>
+#include <cstdio>
+#include <cmath>
+#include <cstring>
+#include <cstdlib>
----------------
These should go into the `.cpp` file.


Repository:
  rT test-suite

https://reviews.llvm.org/D49246





More information about the llvm-commits mailing list