diff mbox

Handle data dependence relations with different bases

Message ID 87lgp38p59.fsf@linaro.org
State New
Headers show

Commit Message

Richard Sandiford June 7, 2017, 9:27 p.m. UTC 
Richard Biener <richard.guenther@gmail.com> writes:
>>> How does this look?  Changes since v1:
>>>
>>> - Added access_fn_component_p to check for valid access function components.
>>>
>>> - Added access_fn_components_comparable_p instead of using
>>>   types_compatibloe_p directly.
>>>
>>> - Added more commentary.
>>>
>>> - Added local structures to represent the sequence, so that it's
>>>   more obvious which variables are temporaries and which aren't.
>>>
>>> - Added the test above to vect-alias-check-3.c.
>>>
>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>
> This is ok.

Thanks.  Just been retesting, and I think I must have forgotten
to include Ada last time.  It turns out that the patch causes a dg-scan
regression in gnat.dg/vect17.adb, because we now think that if the
array RECORD_TYPEs *do* alias in:

   procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
   begin
      for I in Sarray'Range loop
         R(I) := X(I) + Y(I);
      end loop;
   end;

then the dependence distance must be zero.  Eric, does that hold true
for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?  Or are
the arrays allowed to overlap by an arbitrary number of indices?

If the assumption is correct, is the patch below OK?

Thanks,
Richard


2017-06-07  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/testsuite/
	* gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
	* gnat.dg/vect17.adb (Add): Create a dependence distance of 1
	when X = R or Y = R.

Comments

Richard Sandiford June 15, 2017, 1:45 p.m. UTC | #1 
Ping for this Ada patch/question.

Richard Sandiford <richard.sandiford@linaro.org> writes:
> Richard Biener <richard.guenther@gmail.com> writes:
>>>> How does this look?  Changes since v1:
>>>>
>>>> - Added access_fn_component_p to check for valid access function components.
>>>>
>>>> - Added access_fn_components_comparable_p instead of using
>>>>   types_compatibloe_p directly.
>>>>
>>>> - Added more commentary.
>>>>
>>>> - Added local structures to represent the sequence, so that it's
>>>>   more obvious which variables are temporaries and which aren't.
>>>>
>>>> - Added the test above to vect-alias-check-3.c.
>>>>
>>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>>
>> This is ok.
>
> Thanks.  Just been retesting, and I think I must have forgotten
> to include Ada last time.  It turns out that the patch causes a dg-scan
> regression in gnat.dg/vect17.adb, because we now think that if the
> array RECORD_TYPEs *do* alias in:
>
>    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>    begin
>       for I in Sarray'Range loop
>          R(I) := X(I) + Y(I);
>       end loop;
>    end;
>
> then the dependence distance must be zero.  Eric, does that hold true
> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?  Or are
> the arrays allowed to overlap by an arbitrary number of indices?
>
> If the assumption is correct, is the patch below OK?
>
> Thanks,
> Richard
>
>
> 2017-06-07  Richard Sandiford  <richard.sandiford@linaro.org>
>
> gcc/testsuite/
> 	* gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
> 	* gnat.dg/vect17.adb (Add): Create a dependence distance of 1
> 	when X = R or Y = R.
>
> Index: gcc/testsuite/gnat.dg/vect17.ads
> ===================================================================
> --- gcc/testsuite/gnat.dg/vect17.ads	2015-10-14 14:58:56.000000000 +0100
> +++ gcc/testsuite/gnat.dg/vect17.ads	2017-06-07 22:10:24.796368118 +0100
> @@ -1,6 +1,6 @@
>  package Vect17 is
>  
> -   type Sarray is array (1 .. 4) of Long_Float;
> +   type Sarray is array (1 .. 5) of Long_Float;
>     for Sarray'Alignment use 16;
>  
>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray);
> Index: gcc/testsuite/gnat.dg/vect17.adb
> ===================================================================
> --- gcc/testsuite/gnat.dg/vect17.adb	2015-10-14 14:58:56.000000000 +0100
> +++ gcc/testsuite/gnat.dg/vect17.adb	2017-06-07 22:10:24.796368118 +0100
> @@ -5,8 +5,9 @@ package body Vect17 is
>  
>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>     begin
> -      for I in Sarray'Range loop
> -         R(I) := X(I) + Y(I);
> +      R(1) := X(5) + Y(5);
> +      for I in 1 .. 4 loop
> +         R(I + 1) := X(I) + Y(I);
>        end loop;
>     end;
>
Richard Sandiford June 22, 2017, 11:34 a.m. UTC | #2 
Ping*2

Richard Sandiford <richard.sandiford@linaro.org> writes:
> Ping for this Ada patch/question.
>
> Richard Sandiford <richard.sandiford@linaro.org> writes:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>> How does this look?  Changes since v1:
>>>>>
>>>>> - Added access_fn_component_p to check for valid access function
>>>>> components.
>>>>>
>>>>> - Added access_fn_components_comparable_p instead of using
>>>>>   types_compatibloe_p directly.
>>>>>
>>>>> - Added more commentary.
>>>>>
>>>>> - Added local structures to represent the sequence, so that it's
>>>>>   more obvious which variables are temporaries and which aren't.
>>>>>
>>>>> - Added the test above to vect-alias-check-3.c.
>>>>>
>>>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>>>
>>> This is ok.
>>
>> Thanks.  Just been retesting, and I think I must have forgotten
>> to include Ada last time.  It turns out that the patch causes a dg-scan
>> regression in gnat.dg/vect17.adb, because we now think that if the
>> array RECORD_TYPEs *do* alias in:
>>
>>    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>>    begin
>>       for I in Sarray'Range loop
>>          R(I) := X(I) + Y(I);
>>       end loop;
>>    end;
>>
>> then the dependence distance must be zero.  Eric, does that hold true
>> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
>> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?  Or are
>> the arrays allowed to overlap by an arbitrary number of indices?
>>
>> If the assumption is correct, is the patch below OK?
>>
>> Thanks,
>> Richard
>>
>>
>> 2017-06-07  Richard Sandiford  <richard.sandiford@linaro.org>
>>
>> gcc/testsuite/
>> 	* gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
>> 	* gnat.dg/vect17.adb (Add): Create a dependence distance of 1
>> 	when X = R or Y = R.
>>
>> Index: gcc/testsuite/gnat.dg/vect17.ads
>> ===================================================================
>> --- gcc/testsuite/gnat.dg/vect17.ads	2015-10-14 14:58:56.000000000 +0100
>> +++ gcc/testsuite/gnat.dg/vect17.ads	2017-06-07 22:10:24.796368118 +0100
>> @@ -1,6 +1,6 @@
>>  package Vect17 is
>>  
>> -   type Sarray is array (1 .. 4) of Long_Float;
>> +   type Sarray is array (1 .. 5) of Long_Float;
>>     for Sarray'Alignment use 16;
>>  
>>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray);
>> Index: gcc/testsuite/gnat.dg/vect17.adb
>> ===================================================================
>> --- gcc/testsuite/gnat.dg/vect17.adb	2015-10-14 14:58:56.000000000 +0100
>> +++ gcc/testsuite/gnat.dg/vect17.adb	2017-06-07 22:10:24.796368118 +0100
>> @@ -5,8 +5,9 @@ package body Vect17 is
>>  
>>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>>     begin
>> -      for I in Sarray'Range loop
>> -         R(I) := X(I) + Y(I);
>> +      R(1) := X(5) + Y(5);
>> +      for I in 1 .. 4 loop
>> +         R(I + 1) := X(I) + Y(I);
>>        end loop;
>>     end;
>>
Richard Sandiford June 29, 2017, 5:20 p.m. UTC | #3 
Ping*3

Richard Sandiford <richard.sandiford@linaro.org> writes:
> Ping*2
>
> Richard Sandiford <richard.sandiford@linaro.org> writes:
>> Ping for this Ada patch/question.
>>
>> Richard Sandiford <richard.sandiford@linaro.org> writes:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>>> How does this look?  Changes since v1:
>>>>>>
>>>>>> - Added access_fn_component_p to check for valid access function
>>>>>> components.
>>>>>>
>>>>>> - Added access_fn_components_comparable_p instead of using
>>>>>>   types_compatibloe_p directly.
>>>>>>
>>>>>> - Added more commentary.
>>>>>>
>>>>>> - Added local structures to represent the sequence, so that it's
>>>>>>   more obvious which variables are temporaries and which aren't.
>>>>>>
>>>>>> - Added the test above to vect-alias-check-3.c.
>>>>>>
>>>>>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>>>>
>>>> This is ok.
>>>
>>> Thanks.  Just been retesting, and I think I must have forgotten
>>> to include Ada last time.  It turns out that the patch causes a dg-scan
>>> regression in gnat.dg/vect17.adb, because we now think that if the
>>> array RECORD_TYPEs *do* alias in:
>>>
>>>    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>>>    begin
>>>       for I in Sarray'Range loop
>>>          R(I) := X(I) + Y(I);
>>>       end loop;
>>>    end;
>>>
>>> then the dependence distance must be zero.  Eric, does that hold true
>>> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
>>> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?  Or are
>>> the arrays allowed to overlap by an arbitrary number of indices?
>>>
>>> If the assumption is correct, is the patch below OK?
>>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> 2017-06-07  Richard Sandiford  <richard.sandiford@linaro.org>
>>>
>>> gcc/testsuite/
>>> 	* gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
>>> 	* gnat.dg/vect17.adb (Add): Create a dependence distance of 1
>>> 	when X = R or Y = R.
>>>
>>> Index: gcc/testsuite/gnat.dg/vect17.ads
>>> ===================================================================
>>> --- gcc/testsuite/gnat.dg/vect17.ads	2015-10-14 14:58:56.000000000 +0100
>>> +++ gcc/testsuite/gnat.dg/vect17.ads	2017-06-07 22:10:24.796368118 +0100
>>> @@ -1,6 +1,6 @@
>>>  package Vect17 is
>>>  
>>> -   type Sarray is array (1 .. 4) of Long_Float;
>>> +   type Sarray is array (1 .. 5) of Long_Float;
>>>     for Sarray'Alignment use 16;
>>>  
>>>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray);
>>> Index: gcc/testsuite/gnat.dg/vect17.adb
>>> ===================================================================
>>> --- gcc/testsuite/gnat.dg/vect17.adb	2015-10-14 14:58:56.000000000 +0100
>>> +++ gcc/testsuite/gnat.dg/vect17.adb	2017-06-07 22:10:24.796368118 +0100
>>> @@ -5,8 +5,9 @@ package body Vect17 is
>>>  
>>>     procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>>>     begin
>>> -      for I in Sarray'Range loop
>>> -         R(I) := X(I) + Y(I);
>>> +      R(1) := X(5) + Y(5);
>>> +      for I in 1 .. 4 loop
>>> +         R(I + 1) := X(I) + Y(I);
>>>        end loop;
>>>     end;
>>>
Eric Botcazou July 4, 2017, 9:40 a.m. UTC | #4 
[Sorry for missing the previous messages]

> Thanks.  Just been retesting, and I think I must have forgotten
> to include Ada last time.  It turns out that the patch causes a dg-scan
> regression in gnat.dg/vect17.adb, because we now think that if the
> array RECORD_TYPEs *do* alias in:
> 
>    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
>    begin
>       for I in Sarray'Range loop
>          R(I) := X(I) + Y(I);
>       end loop;
>    end;
> 
> then the dependence distance must be zero.  Eric, does that hold true
> for Ada?  I.e. if X and R (or Y and R) alias, must it be the case that
> X(I) can only alias R(I) and not for example R(I-1) or R(I+1)?

Yes, I'd think so (even without the artificial RECORD_TYPE around the arrays).

> 2017-06-07  Richard Sandiford  <richard.sandiford@linaro.org>
> 
> gcc/testsuite/
> 	* gnat.dg/vect17.ads (Sarray): Increase range to 1 .. 5.
> 	* gnat.dg/vect17.adb (Add): Create a dependence distance of 1
> 	when X = R or Y = R.

I think that you need to modify vect15 and vect16 the same way.
diff mbox

Patch

Index: gcc/testsuite/gnat.dg/vect17.ads
===================================================================
--- gcc/testsuite/gnat.dg/vect17.ads	2015-10-14 14:58:56.000000000 +0100
+++ gcc/testsuite/gnat.dg/vect17.ads	2017-06-07 22:10:24.796368118 +0100
@@ -1,6 +1,6 @@ 
 package Vect17 is
 
-   type Sarray is array (1 .. 4) of Long_Float;
+   type Sarray is array (1 .. 5) of Long_Float;
    for Sarray'Alignment use 16;
 
    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray);
Index: gcc/testsuite/gnat.dg/vect17.adb
===================================================================
--- gcc/testsuite/gnat.dg/vect17.adb	2015-10-14 14:58:56.000000000 +0100
+++ gcc/testsuite/gnat.dg/vect17.adb	2017-06-07 22:10:24.796368118 +0100
@@ -5,8 +5,9 @@  package body Vect17 is
 
    procedure Add (X, Y : aliased Sarray; R : aliased out Sarray) is
    begin
-      for I in Sarray'Range loop
-         R(I) := X(I) + Y(I);
+      R(1) := X(5) + Y(5);
+      for I in 1 .. 4 loop
+         R(I + 1) := X(I) + Y(I);
       end loop;
    end;