[PATCH] D26890: SHA1: unroll loop in hashBlock.

[Mehdi AMINI via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Support/SHA1.cpp:30
 
+#define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
+
----------------
Why did you turn it into a macro?


================
Comment at: lib/Support/SHA1.cpp:51
+  z += ((w & (x ^ y)) ^ y) + blk0(i) + 0x5A827999 + rol(v, 5);                 \
+  w = rol(w, 30);
+#define R1(v, w, x, y, z, i)                                                   \
----------------
Why a macro for all the `RX(..)`?


================
Comment at: lib/Support/SHA1.cpp:95-174
+  R0(a, b, c, d, e, 0);
+  R0(e, a, b, c, d, 1);
+  R0(d, e, a, b, c, 2);
+  R0(c, d, e, a, b, 3);
+  R0(b, c, d, e, a, 4);
+  R0(a, b, c, d, e, 5);
+  R0(e, a, b, c, d, 6);
----------------
davide wrote:
> I don't think this is terrible per-se (not quite readable), but @chandlerc pointed out during the DevSummit (at the libc++ performance BOF IIRC) that we should try to avoid unrolling loops by hand in our algorithms (and make sure the compiler does that on our behalf).
> Now, for this case, I'm not sure if LLVM knows how to unroll this loop (and if it doesn't, I'm not sure how profitable it is to teach how to do it), but something to keep in mind in general.
Yes, I wonder how is the codegen impacted if you turn the above into 4 different loops?



https://reviews.llvm.org/D26890