[AArch64], patch] PR71727 fix -mstrict-align

[Christophe Lyon]

ping?

On 20 September 2017 at 15:17, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> Hi,
>
> On 11 September 2017 at 10:45, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Tue, Jul 18, 2017 at 5:50 AM, Christophe Lyon
>> <christophe.lyon@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