Re: [PATCH maint-1.6.5 v2] block-sha1: avoid pointer conversion that violates alignment constraints

From: Michael Cree <mcree@orcon.net.nz>
Date: Jul 15, 2012, 20:58 UTC
Message-ID: <FDFB73BB-64E8-48F8-95DB-94C0EC7FFB2A@orcon.net.nz>
In-Reply-To: <20120714205049.GA28502@burratino>

Jonathan,

Thanks for acting so promptly on this. Just a minor point on the commit message below.

On 15/07/2012, at 8:50 AM, Jonathan Nieder wrote:

Show 7 quoted lines

> Unfortunately, Michael noticed on an Alpha machine that git was using
> plain 32-bit reads anyway.  As soon as we convert a pointer to int *,
> the compiler can assume that the object pointed to is correctly
> aligned as an int (C99 section 6.3.2.3 "pointer conversions"
> paragraph 7), and gcc takes full advantage by converting the get_be32
> calls back to a load and bswap and producing a whole bunch of
> unaligned access traps.

Alpha does not have a bswap (or similar) instruction. I do recall reading somewhere that one can get halfway to swapping endianness by treating a number as a VAX float and converting to IEEE float, with only a couple or so more instructions to achieve the full swap. But I don't think that is available to us anyway because on Debian we compile for generic Alpha. I suspect that the crux of the matter is that compiling for generic Alpha also means that byte access instructions are not permitted. In other words, all memory accesses must be long words (32bits) or quad words (64bits). In the get_be32 routine the compiler _has_ to issue 32bit memory accesses whether the base pointer is char * or int *. If the pointer is unaligned then two neighbouring aligned 32bit loads are required to ensure that all four bytes are loaded. If the pointer is aligned then a single 32bit load gets all the four bytes. Having never looked at the generated assembler code, I nevertheless suspect that is the guts of the optimisation --- the compiler can eliminate an access to memory if it knows the pointer is aligned.

Cheers Michael.