[PATCH] D48046: [test-suite] Backprop kernel from Rodinia Benchmark

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: backprop/backprop.c:13
+#include "backprop.h"
+#define ABS(x) (((x) > 0.0) ? (x) : (-(x)))
+
----------------
This ABS implementation doesn't work correctly for negative zeros, use `fabs()` which is likely to be faster anyway.


================
Comment at: backprop/backprop.c:17-28
+    float *__restrict__ hidden_units, float *__restrict__ output_units,
+    float *__restrict__ hidden_delta, float *__restrict__ output_delta,
+    float *__restrict__ target,
+    float input_weights[LAYERSIZE + 1][HIDDEN_SIZE + 1],
+    float hidden_weights[HIDDEN_SIZE + 1][OUTPUT_SIZE + 1],
+    float input_prev_weights[LAYERSIZE + 1][HIDDEN_SIZE + 1],
+    float hidden_prev_weights[HIDDEN_SIZE + 1][OUTPUT_SIZE + 1],
----------------
Why not convert this to proper C99 so you can use the standardized `restrict` instead of the `__restrict__` compiler extension. That would also allow you to declare variables on assignment instead of the noise early declarations.


================
Comment at: backprop/main.c:153-158
+  input_units = (float(*__restrict__))malloc(((n_in + 1) * sizeof(float)));
+  hidden_units = (float(*__restrict__))malloc(((n_hidden + 1) * sizeof(float)));
+  output_units = (float(*__restrict__))malloc(((n_out + 1) * sizeof(float)));
+  hidden_delta = (float(*__restrict__))malloc(((n_hidden + 1) * sizeof(float)));
+  output_delta = (float(*__restrict__))malloc(((n_out + 1) * sizeof(float)));
+  target = (float(*__restrict__))malloc(((n_out + 1) * sizeof(float)));
----------------
Do you need to cast the malloc results to `restrict`? I would expect the compiler to figure that out on it's own.


Repository:
  rT test-suite

https://reviews.llvm.org/D48046