Re: [v4 PATCH] sha2: new header <sha2.h>

[Shawn Landden <shawn at churchofgit dot com>]

On Thu, Apr 09, 2015 at 11:45:52AM -0700, Richard Henderson wrote:
> On 04/07/2015 06:02 PM, Shawn Landden wrote:
> > +#if _STRING_ARCH_unaligned
> > +
> > +#define put_be32(p, v)	do { *(uint32_t *)(p) = be32toh(v); } while (0)
> > +#define put_be64(p, v)	do { *(uint64_t *)(p) = be64toh(v); } while (0)
> > +
> > +#else
> > +
> > +#define put_be32(p, v)	do { \
> > +	unsigned uint32_t __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)
> > +#define put_be64(p, v)	do { \
> > +	unsigned uint64_t __v = (v); \
> > +	*((unsigned char *)(p) + 0) = __v >> 56; \
> > +	*((unsigned char *)(p) + 1) = __v >> 48; \
> > +	*((unsigned char *)(p) + 2) = __v >> 40; \
> > +	*((unsigned char *)(p) + 3) = __v >> 32; \
> > +	*((unsigned char *)(p) + 4) = __v >> 24; \
> > +	*((unsigned char *)(p) + 5) = __v >> 16; \
> > +	*((unsigned char *)(p) + 6) = __v >>  8; \
> > +	*((unsigned char *)(p) + 7) = __v >>  0; } while (0)
> > +
> > +#endif
> 
> Is there an extremely good reason why you're using macros instead of inline
> functions here?
> 
> Storing through a packed structure is the easiest way to make gcc produce what
> you want, without having to make every port define _STRING_ARCH_unaligned.
> 
> E.g.
> 
> static inline void put_be32(void *p, uint32_t v)
> {
>   struct __attribute__((packed, may_alias)) { uint32_t x; } *u = p;
>   u->x = be32toh(v);
> }
Nice! I assume this could be used for get_unaligned too?

static inline uint32_t get_unaligned_be32(void *p)
{
  struct __attribute__((packed, may_alias)) { uint32_t x; } *u = p;
  return be32toh(u->x);
}

> 
> This should produce significantly better code for any gcc target that normally
> requires alignment but implements the movmem pattern.  Especially Alpha (using
> stq_u) and mips (using swl, swr).
> 
> 
> r~