On Wed, Sep 27, 2017 at 06:25:56PM +0100, Christophe Lyon wrote: > ping?
OK, thanks. Reviewed-by: James Greenhalgh <james.greenha...@arm.com> James > > On 20 September 2017 at 15:17, Christophe Lyon > <christophe.l...@linaro.org> wrote: > > Hi, > > > > On 11 September 2017 at 10:45, Andrew Pinski <pins...@gmail.com> wrote: > >> On Tue, Jul 18, 2017 at 5:50 AM, Christophe Lyon > >> <christophe.l...@linaro.org> wrote: > >>> Hello, > >>> > >>> I've received a complaint that GCC for AArch64 would generate > >>> vectorized code relying on unaligned memory accesses even when using > >>> -mstrict-align. This is a problem for code where such accesses lead to > >>> memory faults. > >>> > >>> A previous patch (r243333) introduced > >>> aarch64_builtin_support_vector_misalignment, which rejects such > >>> accesses when the element size is 64 bits, and accept them otherwise, > >>> which I think it shouldn't. The testcase added at that time only used > >>> 64 bits elements, and therefore didn't fully test the patch. > >>> > >>> The report I received is about vectorized accesses to an array of > >>> unsigned chars, whose start address is not aligned on a 128 bits > >>> boundary. > >>> > >>> The attached patch fixes the problem by making > >>> aarch64_builtin_support_vector_misalignment always return false when > >>> the misalignment is not known at compile time. > >>> > >>> I've also added a testcase, which tries to check if the array start > >>> address alignment is checked (using %16, and-ing with #15), so that > >>> loop peeling is performed *before* using vectorized accesses. Without > >>> the patch, vectorized accesses are used at the beginning of the array, > >>> and byte accesses are used for the remainder at the end, and there is > >>> not such 'and wX,wX,15'. > >>> > >>> BTW, I'm not sure about the same hook for arm... it seems to me it has > >>> a similar problem. > >>> > >>> OK? > >> > >> I would keep part of the comment: > >> - /* Misalignment factor is unknown at compile time but we know > >> - it's word aligned. */ > >> > >> Something like: > >> /* Misalignment factor is unknown at compile time. */ > >> > >> Otherwise it is not obvious what -1 means. > >> > >> Other than I think this patch is good (I cannot approve though). > >> > > > > Here is an updated patch, with the comment added as Andrew suggested. > > > > OK? > > > > Thanks, > > > > Christophe > > > >> Thanks, > >> Andrew > >> > >>> > >>> Thanks, > >>> > >>> Christophe
