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

[Joerg Sonnenberger via llvm-commits]

joerg added inline comments.


================
Comment at: lib/Support/SHA1.cpp:12
+// (http://oauth.googlecode.com/svn/code/c/liboauth/src/sha1.c and
+// https://github.com/jsonn/src/blob/trunk/common/lib/libc/hash/sha1/sha1.c)
 // and modified by wrapping it in a C++ interface for LLVM,
----------------
davide wrote:
> I'd rather link the original NetBSD repo rather than Joerg's mirror (anoncvs.netbsd.org)
Agreed, it was primarily meant as starting point.

http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/common/lib/libc/hash/sha1/sha1.c?rev=1.6

would be a better link.


================
Comment at: lib/Support/SHA1.cpp:30
 
+#define rol(value, bits) (((value) << (bits)) | ((value) >> (32 - (bits))))
+
----------------
davide wrote:
> mehdi_amini wrote:
> > Why did you turn it into a macro?
> Agree, not particularly fond of macros in new software unless really needed.
That's actually adopted from the NetBSD implementation. There is no huge advantage for inline function vs macro; the macro just keeps the diff a bit smaller.


================
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)                                                   \
----------------
mehdi_amini wrote:
> Why a macro for all the `RX(..)`?
The macros reflect the building blocks of the main loop, e.g. the different constants and blocks used.
Again, this could be an inline function with references and hoping the compiler optimised all away, but using a macro keeps the diff down.


https://reviews.llvm.org/D26890