On Thu, 13 Aug 2009, Linus Torvalds wrote:
>
>
> On Thu, 13 Aug 2009, Nicolas Pitre wrote:
> >
> > In addition to X86, PowerPC and S390 are capable of unaligned memory
> > accesses.
> >
> > Signed-off-by: Nicolas Pitre <nico@xxxxxxx>
>
> Ack on all your patches (1-3 + this). Looks fine to me.
>
> I do wonder if we should try to do basically "per-architecture hack
> header-files", and then have for each architecture a small trivial
> 'hack-x86.h' kind of thing that just does
>
> /* x86 hacks */
> #define get_be32(p) ntohl(*(unsigned int *)(p))
> #define put_be32(p, v) do { *(unsigned int *)(p) = htonl(v); } while (0)
> #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
>
> #define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
> #define SHA_ROL(x,n) SHA_ASM("rol", x, n)
> #define SHA_ROR(x,n) SHA_ASM("ror", x, n)
>
> and then we'd have each architecture separated out. Add a few
> "generic helpers":
>
> - be32-generic.h:
>
> #define get_be32(p) ( \
> (*((unsigned char *)(p) + 0) << 24) | \
> (*((unsigned char *)(p) + 1) << 16) | \
> (*((unsigned char *)(p) + 2) << 8) | \
> (*((unsigned char *)(p) + 3) << 0) )
>
> #define put_be32(p, v) do { \
> unsigned int __v = (v); \
> *((unsigned char *)(p) + 0) = __v >> 24; \
> *((unsigned char *)(p) + 1) = __v >> 16; \
> *((unsigned char *)(p) + 2) = __v >> 8; \
> *((unsigned char *)(p) + 3) = __v >> 0; } while (0)
>
> - rotate-generic.h:
>
> #define SHA_ROT(X,l,r) (((X) << (l)) | ((X) >> (r)))
> #define SHA_ROL(X,n) SHA_ROT(X,n,32-(n))
> #define SHA_ROR(X,n) SHA_ROT(X,32-(n),n)
>
> that architectures could use when they want to use some particular
> portable version. Then, add a final "hack-generic.h" with the fallback
> cases that just does
>
> #include "be32-generic.h"
> #include "rotate-generic.h"
> #define setW(x,val) (W(x) = (val))
>
> and you'd have all the hacks separated out and fairly easily used by
> different architectures.. (ie the ARM version would just look like
>
> - hack-arm.h:
>
> #include "be32-generic.h"
> #include "rotate-generic.h"
> #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
>
> and you'd be all done.
>
> Hmm? I don't know if this kind of generalization is strictly needed, so
> I'm just throwing it out as an idea.
Well... first I don't think there are much else we can do with that
code, i.e. there is probably not so many more hacks to add if any. With
my last patch I consider this ready for general consumption.
And given that the only user is this very sha1 implementation then there
is not much value in abstracting them in header files.
And the point of patch 2/3 was to move hack variants for the same
purpose together making it easier to document and understand (and
possibly modify) them. Your suggestion would go in the opposite
direction entirely.
As it is now, I was about to suggest:
git mv block-sha1/sha1.[ch] .
rmdir block-sha1
rm -r mozilla-sha1
rm -r arm
rm -r ppc
and remove support for openssl's SHA1 usage, making this implementation
unconditional. After all it is faster, or so close to be faster than
the alternatives, that we should probably cut on the extra dependency
and simplify portability issues at the same time.
Nicolas
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html