Re: [patch] Fix PR tree-optimization/49960 ,Fix self data dependence

[Razya Ladelsky <RAZYA at il dot ibm dot com>]

gcc-patches-owner@gcc.gnu.org wrote on 17/10/2011 09:03:59 AM:

> From: Richard Guenther <richard.guenther@gmail.com>
> To: Razya Ladelsky/Haifa/IBM@IBMIL
> Cc: gcc-patches@gcc.gnu.org, Sebastian Pop <spop@gcc.gnu.org>
> Date: 17/10/2011 09:04 AM
> Subject: Re: [patch] Fix PR tree-optimization/49960 ,Fix self data 
dependence
> Sent by: gcc-patches-owner@gcc.gnu.org
> 
> On Mon, Oct 17, 2011 at 8:23 AM, Razya Ladelsky <RAZYA@il.ibm.com> 
wrote:
> > This patch fixes the failures described in
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49960
> > It also fixes bzips when run with autopar enabled.
> >
> > In both cases the self dependences are not handled correctly.
> > In the first case, a non affine access is analyzed:
> > in the second, the distance vector is not calculated correctly (the
> > distance vector considered for for self dependences is always 
(0,0,...))
> >
> > As  a result, the loops get wrongfully parallelized.
> >
> > The patch avoids the special handling of  self dependences, and 
analyzes
> > all dependences in the same way. Specific adjustments
> > and support for the self dependence cases were made.
> 
> Can you elaborate on
> 
> @@ -3119,8 +3135,11 @@ add_other_self_distances (struct 
data_dependence_r
>         {
>           if (DDR_NUM_SUBSCRIPTS (ddr) != 1)
>        {
> -        DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
> -        return;
> +        if (DDR_NUM_SUBSCRIPTS (ddr) != 2 || !integer_zerop 
(DR_ACCESS_FN
> (DDR_A (ddr), 1)))
> +          {
> +            DDR_ARE_DEPENDENT (ddr) = chrec_dont_know;
> +            return;
> +          }
>        }
> 
>           access_fun = DR_ACCESS_FN (DDR_A (ddr), 0);
> 
> ?  It needed a comment before, and now so even more.
> 
> The rest of the patch is ok, I suppose the above hunk is to enhance
> something, not
> to fix the bug?

For fortran code like:

      DO 140 J=1,MB
         DO 130 K=1,NA
            BKJ=B(K,J)
            IF(BKJ.EQ.ZERO) GO TO 130
               DO 120 I=1,MA
                  C(I,J)=C(I,J)+A(K,I)*BKJ
  120          CONTINUE
  130    CONTINUE
  140 CONTINUE
      RETURN


The access functions for the C(i j) self dependence are:

(Data Dep: 
#(Data Ref: 
#  bb: 9 
#  stmt: D.1427_79 = *c_78(D)[D.1426_77];
#  ref: *c_78(D)[D.1426_77];
#  base_object: *c_78(D);
#  Access function 0: {{(stride.12_25 + 1) + offset.13_36, +, 
stride.12_25}_1, +, 1}_3
#  Access function 1: 0B
#)
#(Data Ref: 
#  bb: 9 
#  stmt: *c_78(D)[D.1426_77] = D.1433_88;
#  ref: *c_78(D)[D.1426_77];
#  base_object: *c_78(D);
#  Access function 0: {{(stride.12_25 + 1) + offset.13_36, +, 
stride.12_25}_1, +, 1}_3
#  Access function 1: 0B
#)


Two dimesions are created to describe C(i j) although there's no need for 
access function 1 which is just 0B.


If this was a C code, we would have these two access functions for 
C[i][j]:

(Data Dep: 
#(Data Ref: 
#  bb: 5 
#  stmt: t_10 = C[i_33][j_37];
#  ref: C[i_33][j_37];
#  base_object: C
#  Access function 0: {3, +, 1}_3
#  Access function 1: {3, +, 1}_2
#)
#(Data Ref: 
#  bb: 5 
#  stmt: C[i_33][j_37] = D.3852_15;
#  ref: C[i_33][j_37];
#  base_object: C
#  Access function 0: {3, +, 1}_3
#  Access function 1: {3, +, 1}_2
#)


In order to handle the Fortran data accesses, even for simple cases as 
above, 
I would need to handle multivariate accesses.
The data dependence analysis doesn't know how to handle such dependences 
if there's more than one subscript.
The above Frotran code doesn't actually have two subscripts, but one, and 
thus should be handled.
 
The reason this issue came up only with the changes of this patch is that 
now 
add_other_self_distances is called from build_classic_dist_vector, which 
is called also for self dependences.
Before the patch, the distance vector for self dependences was always 
determined as a vector of 0's, and build_classic_dist_vector
was not called.

I hope it's clearer now, I will add a comment to the code, and submit it 
before committing it.

Thanks,
Razya


> 
> Thanks,
> Richard.
> 
> > Bootstrap and testsuite pass successfully for ppc64-redhat-linux.
> >
> > OK for trunk?
> > Thank you,
> > Razya
> >
> >
> > ChangeLog:
> >
> >        PR tree-optimization/49960
> >        * tree-data-ref.c (compute_self_dependence): Remove.
> >             (initialize_data_dependence_relation): Add intializations.
> > Remove compute_self_dependence.
> >             (add_other_self_distances): Add support for two dimensions 
if
> > the second is zero.
> >             (compute_affine_dependence): Remove the 
!DDR_SELF_REFERENCE
> > condition.
> >             (compute_all_dependences): Remove call to
> > compute_self_dependence. Add call to compute_affine_dependence
> 
> > testsuite/ChangeLog:
> >
> >        PR tree-optimization/49660
> >        * gcc.dg/autopar/pr49660.c: New test.
> >           * gcc.dg/autopar/pr49660-1.c: New test.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >