Re: patch for bug 42640

[Sebastian Pop <sebpop at gmail dot com>]

On Mon, Mar 1, 2010 at 12:46, Yazdani, Reza <Reza.Yazdani@amd.com> wrote:
> Problem description and Fix for bug 42640:
>
> The loop distribution is done using SSA forms and in this case the SSA of the original loop is not propagated correctly to the distributed loops. Here is the loop with problem.
>
>
> ? ? ?for (iloop = 1; iloop <= 2; iloop++)
> ? ? ? ?{
> ? ? ? ? ?rr_node[inode].a = i;
> ? ? ? ? ?rr_node[inode].b = j;
> ? ? ? ? ?rr_node[inode].c = ipad;
>
> ? ? ? ? ?inode = p_node;
> ? ? ? ?}
>
> inode has a value of 0 at the loop entry (see attached pr42640.c). It has the value of p_node in the second iteration. The intermediate code for this loop before distribution is:
>
> ?loop_2 (header = 5, latch = 6, niter = , upper_bound = 2, estimate = 2)
> ?{
> ? ?bb_5 (preds = {bb_6 bb_4 }, succs = {bb_6 bb_7 })
> ? ?{
> ? ?<bb 5>:
> ? ? ?# inode_18 = PHI <p_node_6(6), ipad_17(4)>
> ? ? ?# iloop_20 = PHI <iloop_25(6), 1(4)>
> ? ? ?# .MEM_21 = PHI <.MEM_32(6), .MEM_15(4)>
> ? ? ?D.3359_11 = (long unsigned int) inode_18;
> ? ? ?D.3360_12 = D.3359_11 * 12;
> ? ? ?D.3361_13 = pretmp.10_35 + D.3360_12;
> ? ? ?# .MEM_30 = VDEF <.MEM_21>
> ? ? ?D.3361_13->a = i_14(D);
> ? ? ?# .MEM_31 = VDEF <.MEM_30>
> ? ? ?D.3361_13->b = j_19(D);
> ? ? ?# .MEM_32 = VDEF <.MEM_31>
> ? ? ?D.3361_13->c = ipad_17;
> ? ? ?iloop_25 = iloop_20 + 1;
> ? ? ?if (iloop_25 <= 2)
> ? ? ? ?goto <bb 6>;
> ? ? ?else
> ? ? ? ?goto <bb 7>;
>
> ? ?}
>
> "inode_18 = PHI <p_node_6(6), ipad_17(4)>" is the PHI function describing the induction variable.
>
> After the distribution the first loop is (see after_dis.txt):
>
> ?loop_4 (header = 13, latch = 14, niter = )
> ?{
> ? ?bb_13 (preds = {bb_14 bb_12 }, succs = {bb_14 bb_15 })
> ? ?{
> ? ?<bb 13>:
> ? ? ?# inode_44 = PHI <inode_44(14), ipad_17(12)>
> ? ? ?# iloop_45 = PHI <iloop_53(14), 1(12)>
> ? ? ?# .MEM_55 = PHI <.MEM_51(14), .MEM_23(12)>
> ? ? ?D.3359_47 = (long unsigned int) inode_44;
> ? ? ?D.3360_48 = D.3359_47 * 12;
> ? ? ?D.3361_49 = pretmp.10_35 + D.3360_48;
> ? ? ?# .MEM_51 = VDEF <.MEM_55>
> ? ? ?D.3361_49->b = j_19(D);
> ? ? ?iloop_53 = iloop_45 + 1;
> ? ? ?if (iloop_53 <= 2)
> ? ? ? ?goto <bb 14>;
> ? ? ?else
> ? ? ? ?goto <bb 15>;
>
> ? ?}
> ? ?bb_14 (preds = {bb_13 }, succs = {bb_13 })
> ? ?{
> ? ?<bb 14>:
> ? ? ?goto <bb 13>;
>
> ? ?}
> ?}
>
> Induction variable is:"# inode_44 = PHI <inode_44(14), ipad_17(12)>"
>
> Note that inode_44 is never initialized in the outer loop.
> ---------------------------------------------------------------------------
>
> This patch initialize the PHI functions for indexed functions assigned to an invariant variables in a distributed loops to the value of the PHI function used in the original loop.
>
> Suggested ChangeLog entry:
>
> "2010-02-26 ?Reza Yazdani ?<reza.yazdani@amd.com>
>
> ? ? ? ?PR middle-end/42640
> ? ? ? ?* (tree-loop-distribution.c): replaced the assignment
> ? ? ? ? ?from new induction variable to the assignment of the
> ? ? ? ?value from the original loop PHI function."
>
> Passed bootstrap and tested with no regressions the testsuite on amd64-linux.
>

This looks good to me, but I cannot approve it.
Richi, could you also have a look at this patch?

Thanks,
Sebastian Pop
--
AMD / Open Source Compiler Engineering / GNU Tools