Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Jun 26, 2017 at 1:50 PM, Richard Sandiford
> <richard.sandif...@linaro.org> wrote:
>> Richard Biener <richard.guent...@gmail.com> writes:
>>> On Mon, Jun 26, 2017 at 1:14 PM, Richard Sandiford
>>> <richard.sandif...@linaro.org> wrote:
>>>> I don't think the problem is the lack of a cap. In the test case we
>>>> see that:
>>>>
>>>> 1. B is known at compile time to be X * vecsize + Y when considered in
>>>> isolation, because the base alignment derived from its DR_REF >=
>>>> vecsize.
>>>> So DR_MISALIGNMENT (B) == Y.
>>>>
>>>> 2. A's misalignment wrt vecsize is not known at compile time when
>>>> considered in isolation, because no useful base alignment can be
>>>> derived from its DR_REF. (The DR_REF is to a plain int rather than
>>>> to a structure with a high alignment.) So DR_MISALIGNMENT (A) == -1.
>>>>
>>>> 3. A and B when considered as a pair trivially have the same misalignment
>>>> wrt vecsize, for the reasons above.
>>>>
>>>> Each of these results is individually correct. The problem is that the
>>>> assert is conflating two things: it's saying that if we know two datarefs
>>>> have the same misaligment, we must either be able to calculate a
>>>> compile-time misalignment for both datarefs in isolation, or we must
>>>> fail to calculate a compile-time misalignment for both datarefs in
>>>> isolation. That isn't true: it's valid to have situations in which the
>>>> compile-time misalignment is known for one dataref in isolation but not
>>>> for the other.
>>>
>>> True. So the assert should then become
>>>
>>> gcc_assert (! known_alignment_for_access_p (dr)
>>> || DR_MISALIGNMENT (dr) / dr_size ==
>>> DR_MISALIGNMENT (dr_peel) / dr_peel_size);
>>>
>>> ?
>>
>> I think it would need to be:
>>
>> gcc_assert (!known_alignment_for_access_p (dr)
>> || !known_alignment_for_access_p (dr_peel)
>> || (DR_MISALIGNMENT (dr) / dr_size
>> == DR_MISALIGNMENT (dr_peel) / dr_peel_size));
>
> I think for !known_alignment_for_access_p (dr_peel) the assert doesn't make
> any sense (DR_MISALIGNMENT is -1), so yes, you are right.
>
>> But yeah, that would work too. The idea with the assert in the patch was
>> that for unconditional references we probably *do* want to try to compute
>> the same compile-time misalignment, but for optimisation reasons rather
>> than correctness. Maybe that's more properly a gcc_checking_assert
>> though, since nothing goes wrong if it fails. So perhaps:
>
> We shouldn't have asserts for optimization reasons, even with checking
> IMHO.
OK.
>> gcc_checking_assert (DR_IS_CONDITIONAL_IN_STMT (dr)
>> || DR_IS_CONDITIONAL_IN_STMT (dr_peel)
>> || (known_alignment_for_access_p (dr)
>> == known_alignment_for_access_p (dr_peel)));
>>
>> as a follow-on assert.
>>
>> Should I split it into two patches, one to change the gcc_assert and
>> another to add the optimisation?
>
> Yes please.
Here's the patch to relax the assert. I'll post the rest in a new thread.
Tested as before. OK to install?
Thanks,
Richard
2017-06-28 Richard Sandiford <richard.sandif...@linaro.org>
gcc/
PR tree-optimization/81136
* tree-vect-data-refs.c (vect_update_misalignment_for_peel): Only
assert that two references with the same misalignment have the same
compile-time misalignment if those compile-time misalignments
are known.
gcc/testsuite/
PR tree-optimization/81136
* gcc.dg/vect/pr81136.c: New test.
Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c 2017-06-26 19:41:19.549571836 +0100
+++ gcc/tree-vect-data-refs.c 2017-06-28 14:25:58.811888377 +0100
@@ -906,8 +906,10 @@ vect_update_misalignment_for_peel (struc
{
if (current_dr != dr)
continue;
- gcc_assert (DR_MISALIGNMENT (dr) / dr_size ==
- DR_MISALIGNMENT (dr_peel) / dr_peel_size);
+ gcc_assert (!known_alignment_for_access_p (dr)
+ || !known_alignment_for_access_p (dr_peel)
+ || (DR_MISALIGNMENT (dr) / dr_size
+ == DR_MISALIGNMENT (dr_peel) / dr_peel_size));
SET_DR_MISALIGNMENT (dr, 0);
return;
}
Index: gcc/testsuite/gcc.dg/vect/pr81136.c
===================================================================
--- /dev/null 2017-06-28 07:28:02.991792729 +0100
+++ gcc/testsuite/gcc.dg/vect/pr81136.c 2017-06-28 14:25:58.810888422 +0100
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+
+struct __attribute__((aligned (32)))
+{
+ char misaligner;
+ int foo[100];
+ int bar[100];
+} *a;
+
+void
+fn1 (int n)
+{
+ int *b = a->foo;
+ for (int i = 0; i < n; i++)
+ a->bar[i] = b[i];
+}
