Re: [PATCH] Fix PR tree-optimization/71170

[Kugan Vivekanandarajah]

Hi Martin,

Thanks for the fix. Just to elaborate (as mentioned in PR)

At tree-ssa-reassoc.c:3897, we have:

stmt:
_15 = _4 + c_7(D);

oe->op def_stmt:
_17 = c_7(D) * 3;


<bb 2>:
a1_6 = s_5(D) * 2;
_1 = (long int) a1_6;
x1_8 = _1 + c_7(D);
a2_9 = s_5(D) * 4;
_2 = (long int) a2_9;
a3_11 = s_5(D) * 6;
_3 = (long int) a3_11;
_16 = x1_8 + c_7(D);
_18 = _1 + _2;
_4 = _16 + _2;
_15 = _4 + c_7(D);
_17 = c_7(D) * 3;
x_13 = _15 + _3;
return x_13;


The root cause of this the place in which we are adding (_17 = c_7(D)
* 3). Finding the right place is not always straightforward as this
case shows.

We could try  Martin Liška's approach, We could also move _17 = c_7(D)
* 3; at tree-ssa-reassoc.c:3897 satisfy the gcc_assert. We could do
this based on the use count of _17.


This patch does this. I have no preferences. Any thoughts ?


Thanks,
Kugan



On 19 May 2016 at 18:04, Martin Liška <mli...@suse.cz> wrote:
> Hello.
>
> Following patch fixes the ICE as it defensively finds the right
> place where a new STMT should be inserted.
>
> Patch bootstraps on x86_64-linux-gnu and no new regression is introduced.
>
> Ready for trunk?
> Thanks,
> Martin

diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 3b5f36b..11beb28 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -3830,6 +3830,29 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
            }
          else
            {
+             /* Change the position of newly added stmt.  */
+             if (TREE_CODE (oe1->op) == SSA_NAME
+                 && has_zero_uses (oe1->op)
+                 && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT 
(oe1->op)))
+               {
+                 gimple *def_stmt = SSA_NAME_DEF_STMT (oe1->op);
+                 gimple_stmt_iterator gsi = gsi_for_stmt (def_stmt);
+                 gsi_remove (&gsi, true);
+                 gsi = gsi_for_stmt (stmt);
+                 gsi_insert_before (&gsi, def_stmt, GSI_SAME_STMT);
+               }
+
+             /* Change the position of newly added stmt.  */
+             if (TREE_CODE (oe2->op) == SSA_NAME
+                 && has_zero_uses (oe2->op)
+                 && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT 
(oe2->op)))
+               {
+                 gimple *def_stmt = SSA_NAME_DEF_STMT (oe2->op);
+                 gimple_stmt_iterator gsi = gsi_for_stmt (def_stmt);
+                 gsi_remove (&gsi, true);
+                 gsi = gsi_for_stmt (stmt);
+                 gsi_insert_before (&gsi, def_stmt, GSI_SAME_STMT);
+               }
              gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op)
                                   == stmt);
              gimple_assign_set_rhs1 (stmt, oe1->op);
@@ -3894,6 +3917,17 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
        }
       else
        {
+         /* Change the position of newly added stmt.  */
+         if (TREE_CODE (oe->op) == SSA_NAME
+             && has_zero_uses (oe->op)
+             && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT 
(oe->op)))
+           {
+             gimple *def_stmt = SSA_NAME_DEF_STMT (oe->op);
+             gimple_stmt_iterator gsi = gsi_for_stmt (def_stmt);
+             gsi_remove (&gsi, true);
+             gsi = gsi_for_stmt (stmt);
+             gsi_insert_before (&gsi, def_stmt, GSI_SAME_STMT);
+           }
          gcc_checking_assert (find_insert_point (stmt, new_rhs1, oe->op)
                               == stmt);
          gimple_assign_set_rhs1 (stmt, new_rhs1);