[PATCH] D47675: [test-suite][RFC] Using Google Benchmark Library on Harris Kernel

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Did you consider putting the driver code (initialization, malloc, free, checking result), Into a different file than the kernel code? This would allow measuring it (code size, LLVM statistics, compile time, etc.) independently from the boilerplate code.



================
Comment at: MicroBenchmarks/harris/harris.cpp:177-197
+  img = (float *)imag;
+  float *Ix;
+  Ix = (float *)(malloc((sizeof(float) * ((2 + R) * (2 + C)))));
+  float *Iy;
+  Iy = (float *)(malloc((sizeof(float) * ((2 + R) * (2 + C)))));
+  float *Ixx;
+  Ixx = (float *)(malloc((sizeof(float) * ((2 + R) * (2 + C)))));
----------------
Maybe the malloc/free calls should be taken out of the measured kernel.


================
Comment at: MicroBenchmarks/harris/harris.cpp:201-208
+      Iy[((_i0 * (2 + C)) + _i1)] =
+          ((((((img[(((-1 + _i0) * (C + 2)) + (-1 + _i1))] *
+                -0.0833333333333f) +
+               (img[(((-1 + _i0) * (C + 2)) + (1 + _i1))] * 0.0833333333333f)) +
+              (img[((_i0 * (C + 2)) + (-1 + _i1))] * -0.166666666667f)) +
+             (img[((_i0 * (C + 2)) + (1 + _i1))] * 0.166666666667f)) +
+            (img[(((1 + _i0) * (C + 2)) + (-1 + _i1))] * -0.0833333333333f)) +
----------------
Could you rewrite this to use multidimensional access subscripts? E.g. `img[_i0-1][_i1-1]`.


================
Comment at: MicroBenchmarks/harris/sha1.hpp:1-45
+/*
+    sha1.hpp - header of
+
+    ============
+    SHA-1 in C++
+    ============
+
----------------
There is already hashing used by test-suite (see `HashProgramOutput.sh`), why add another one?


Repository:
  rT test-suite

https://reviews.llvm.org/D47675