[PATCH] D53030: [MicroBenchmark] Add initial LoopInterchange test/benchmark.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: MicroBenchmarks/LoopInterchange/main.cpp:19
+
+static int test1(int M, int N) {
+
----------------
MatzeB wrote:
> `M` and `N` are unused?
Fixed, looks like I uploaded an old version of the diff.


================
Comment at: MicroBenchmarks/LoopInterchange/main.cpp:58-59
+    int y = test1(M, N);
+    if (state.range(0) == 20)
+      printf("%d\n", y);
+  }
----------------
MatzeB wrote:
> Is the `printf` here a good idea? You'll artificially make one iteration slower which could possibly make the benchmark library take longer to stabilize until the result is statistically significant.
I've added a comment to the code. It's never called with state.range(0), so the printf should be never executed, but ensure the test1 calls cannot be optimized away.


================
Comment at: MicroBenchmarks/LoopInterchange/main.cpp:64
+
+BENCHMARK(BENCHMARK_LI1)
+    ->RangeMultiplier(2)
----------------
MatzeB wrote:
> Does this need to be enclosed in `#ifdef BENCHMARK_LIB` as well? Or alternatively could you just drop all the `#ifdefs` to simplify the code?
I've removed them all.


https://reviews.llvm.org/D53030