This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH]: Fix PR tree-optimization/18792
- From: Sebastian Pop <sebastian dot pop at cri dot ensmp dot fr>
- To: Daniel Berlin <dberlin at dberlin dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 13 Dec 2004 14:50:37 +0100
- Subject: Re: [PATCH]: Fix PR tree-optimization/18792
- References: <Pine.LNX.4.60.0412081856070.29950@dberlin.org>
On Wed, Dec 08, 2004 at 07:00:12PM -0500, Daniel Berlin wrote:
> It turns out we can't depend on loop index numbers being in any particular
> order.
> When it didn't happen, like the testcase in 18792,
>
> This fixes it to use loop depth as the index into the distance and
> direction vectors when building them, as well as fixing the parts of
> tree-loop-linear that indexed into those arrays
> This should work, as the function already assumes there are no sibling
> loops in the nest, etc.
>
> Bootstrapped and regtested on i686-pc-linux-gnu.
>
> Okay for mainline?
>
> 2004-12-07 Daniel Berlin <dberlin@dberlin.org>
>
> Fix PR tree-optimization/18792
>
> * tree-data-ref.c (build_classic_dist_vector): Change first_loop
> to first_loop_depth, and use loop depth instead of loop number.
> (build_classic_dir_vector): Ditto.
> (compute_data_dependences_for_loop): Use depth, not loop number.
> * tree-loop-linear.c (try_interchange_loops): Use loop depth, not
> loop
> number. Pass in loops, instead of loop numbers.
> (gather_interchange_stats): Ditto.
> (linear_transform_loops): Ditto.
>
This patch looks okay, except some minor improvements, see below.
> Index: tree-data-ref.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-data-ref.c,v
> retrieving revision 2.17
> diff -u -p -r2.17 tree-data-ref.c
> --- tree-data-ref.c 12 Nov 2004 00:13:06 -0000 2.17
> +++ tree-data-ref.c 8 Dec 2004 23:54:05 -0000
[...]
> @@ -1862,7 +1861,7 @@ build_classic_dist_vector (struct data_d
> | endloop_1
> In this case, the dependence is carried by loop_1. */
> loop_nb = loop_nb_a < loop_nb_b ? loop_nb_a : loop_nb_b;
> - loop_nb -= first_loop;
> + loop_nb = current_loops->parray[loop_nb]->depth - first_loop_depth;
>
Wouldn't it be more clear to replace s/loop_nb/loop_depth/ in the line
above?
> @@ -1913,8 +1912,8 @@ build_classic_dist_vector (struct data_d
> /* Get the common ancestor loop. */
> lca = find_common_loop (loop_a, loop_b);
>
> - lca_nb = lca->num;
> - lca_nb -= first_loop;
> + lca_nb = lca->depth;
> + lca_nb -= first_loop_depth;
Same here: s/lca_nb/least_common_ancestor_depth/
> @@ -2038,7 +2036,7 @@ build_classic_dir_vector (struct data_de
> | endloop_1
> In this case, the dependence is carried by loop_1. */
> loop_nb = loop_nb_a < loop_nb_b ? loop_nb_a : loop_nb_b;
> - loop_nb -= first_loop;
> + loop_nb = current_loops->parray[loop_nb]->depth - first_loop_depth;
>
Same here: s/loop_nb/loop_depth/
> @@ -2098,7 +2096,7 @@ build_classic_dir_vector (struct data_de
>
> /* Get the common ancestor loop. */
> lca = find_common_loop (loop_a, loop_b);
> - lca_nb = lca->num - first_loop;
> + lca_nb = lca->depth - first_loop_depth;
>
Same here: s/lca_nb/least_common_ancestor_depth/