[Mesa-dev] [PATCH 1/2] util: Add util_memcpy_cpu_to_le()

[Michel Dänzer]

First of all, I think this should be two patches, one which adds the
helper and one which makes radeonsi use it.

On 16.07.2014 01:19, Tom Stellard wrote:
> diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h
> index b9ed197..cd3cf04 100644
> --- a/src/gallium/auxiliary/util/u_math.h
> +++ b/src/gallium/auxiliary/util/u_math.h
> @@ -812,6 +812,28 @@ util_bswap16(uint16_t n)
>            (n << 8);
>  }
>  
> +static INLINE void*
> +util_memcpy_cpu_to_le(void *dest, void *src, size_t n)

This should indicate the size of the words being byte-swapped, e.g.
util_memcpy_cpu_to_le32().


> +{
> +#ifdef PIPE_ARCH_BIG_ENDIAN
> +	size_t i, e;
> +	for (i = 0, e = n % 8; i < e; i++) {
> +		char *d = (char*)dest;
> +		char *s = (char*)src;
> +		d[i] = s[e - i - 1];
> +	}
> +	dest += i;
> +	n -= i;

This doesn't make sense. It's a caller error if n is not a multiple of
the size of the words being being byte-swapped, so maybe just:

	assert((n % 3) == 0);


> +	for (i = 0, e = n / 8; i < e; i++) {
> +		uint64_t *d = (uint64_t*)dest;
> +		uint64_t *s = (uint64_t*)src;
> +		d[i] = util_bswap64(s[e - i - 1]);
> +	}

This looks wrong as well. I think it should be:

	for (i = 0; i < n; i++) {
		uint32_t *d = (uint32_t*)dest;
		uint32_t *s = (uint32_t*)src;
		d[i] = util_bswap32(s[i]);
	}


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer