diff mbox

[2/3,graphite] add array access function in the right order

Message ID 1449685589-10628-2-git-send-email-s.pop@samsung.com
State New
Headers show

Commit Message

Sebastian Pop Dec. 9, 2015, 6:26 p.m. UTC
we used to add the access functions in the wrong order, Fortran style, leading to unprofitable interchanges.
---
 gcc/graphite-sese-to-poly.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom de Vries April 9, 2016, 4 p.m. UTC | #1
On 09/12/15 19:26, Sebastian Pop wrote:
> we used to add the access functions in the wrong order, Fortran style, leading to unprofitable interchanges.
> ---
>   gcc/graphite-sese-to-poly.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
> index 887c212..480c552 100644
> --- a/gcc/graphite-sese-to-poly.c
> +++ b/gcc/graphite-sese-to-poly.c
> @@ -912,7 +912,7 @@ pdr_add_memory_accesses (isl_map *acc, dr_info &dri)
>     for (i = 0; i < nb_subscripts; i++)
>       {
>         isl_pw_aff *aff;
> -      tree afn = DR_ACCESS_FN (dr, nb_subscripts - 1 - i);
> +      tree afn = DR_ACCESS_FN (dr, i);
>
>         aff = extract_affine (scop, afn,
>   			    isl_space_domain (isl_map_get_space (acc)));
>

Hi,

I think the fix for PR68953 ( 
https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=234851 ) undoes 
this fix.

What problem was this patch intending to address?

In other words, have I re-introduced a problem by fixing PR68953 ?

Thanks,
- Tom
Sebastian Pop April 11, 2016, 2:26 p.m. UTC | #2
On Sat, Apr 9, 2016 at 11:00 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 09/12/15 19:26, Sebastian Pop wrote:
>>
>> we used to add the access functions in the wrong order, Fortran style,
>> leading to unprofitable interchanges.
>> ---
>>   gcc/graphite-sese-to-poly.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
>> index 887c212..480c552 100644
>> --- a/gcc/graphite-sese-to-poly.c
>> +++ b/gcc/graphite-sese-to-poly.c
>> @@ -912,7 +912,7 @@ pdr_add_memory_accesses (isl_map *acc, dr_info &dri)
>>     for (i = 0; i < nb_subscripts; i++)
>>       {
>>         isl_pw_aff *aff;
>> -      tree afn = DR_ACCESS_FN (dr, nb_subscripts - 1 - i);
>> +      tree afn = DR_ACCESS_FN (dr, i);
>>
>>         aff = extract_affine (scop, afn,
>>                             isl_space_domain (isl_map_get_space (acc)));
>>
>
> Hi,
>
> I think the fix for PR68953 (
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=234851 ) undoes this
> fix.
>
> What problem was this patch intending to address?
>
> In other words, have I re-introduced a problem by fixing PR68953 ?
>

From the commit message of r234851, it was an interchange profitability
that triggered the change.  I think there is still a problem in the way we are
parsing arrays in C vs. Fortran.  It is good that now we have the correctness
testcase that you added with the patch.

Thanks,
Sebastian
diff mbox

Patch

diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 887c212..480c552 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -912,7 +912,7 @@  pdr_add_memory_accesses (isl_map *acc, dr_info &dri)
   for (i = 0; i < nb_subscripts; i++)
     {
       isl_pw_aff *aff;
-      tree afn = DR_ACCESS_FN (dr, nb_subscripts - 1 - i);
+      tree afn = DR_ACCESS_FN (dr, i);
 
       aff = extract_affine (scop, afn,
 			    isl_space_domain (isl_map_get_space (acc)));