Re: [PATCH2][PR71252] Fix insertion point of stmt_to_insert
On Mon, May 30, 2016 at 2:35 AM, kugan wrote: > > > On 28/05/16 01:28, Kugan Vivekanandarajah wrote: >> >> Hi Richard, >> >> This fix insertion point of stmt_to_insert based on your comments. In >> insert_stmt_before_use , I now use find_insert_point such that we >> insert the stmt_to_insert after its operands are defined. This means >> that we now have to insert before and insert after in other cases. >> >> I also factored out uses of insert_stmt_before_use. >> >> I tested this with: >> ./build/gcc/f951 cp2k_single_file.f90 -O3 -ffast-math -march=westmere >> >> I am running bootstrap and regression testing on x86-64-linux gnu. Is >> this OK for trunk if the testing is fine ? I will also test with other >> test cases from relevant PRs >> >> > Hi, > > Here is the tested patch. I made a slight change to reflect the paten used > everywhere else in reassoc. > > i.e., in insert_stmt_before_use, after finding the insertion point I now do: > > + if (stmt == insert_point) > +gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT); > + else > +insert_stmt_after (stmt_to_insert, insert_point); > > This is consistent with what is done in other places. > > I tested the patch with CPU2006 and bootstrapped and regression tested for > x86-64-none-linux-gnu with no new regressions. > > Is this OK for trunk? Ok. Thanks, Richard. > Thanks, > Kugan > > gcc/testsuite/ChangeLog: > > 2016-05-28 Kugan Vivekanandarajah > > PR middle-end/71269 > PR middle-end/71292 > * gcc.dg/tree-ssa/pr71269.c: New test. > * gcc.dg/tree-ssa/pr71292.c: New test. > > gcc/ChangeLog: > > 2016-05-28 Kugan Vivekanandarajah > > PR middle-end/71269 > PR middle-end/71252 > > * tree-ssa-reassoc.c (insert_stmt_before_use): Use find_insert_point > so > that inserted stmt will not dominate stmts that defines its operand. > (rewrite_expr_tree): Add stmt_to_insert before adding the use stmt. > (rewrite_expr_tree_parallel): Likewise.
Re: [PATCH2][PR71252] Fix insertion point of stmt_to_insert
On 28/05/16 01:28, Kugan Vivekanandarajah wrote:
Hi Richard,
This fix insertion point of stmt_to_insert based on your comments. In
insert_stmt_before_use , I now use find_insert_point such that we
insert the stmt_to_insert after its operands are defined. This means
that we now have to insert before and insert after in other cases.
I also factored out uses of insert_stmt_before_use.
I tested this with:
./build/gcc/f951 cp2k_single_file.f90 -O3 -ffast-math -march=westmere
I am running bootstrap and regression testing on x86-64-linux gnu. Is
this OK for trunk if the testing is fine ? I will also test with other
test cases from relevant PRs
Hi,
Here is the tested patch. I made a slight change to reflect the paten
used everywhere else in reassoc.
i.e., in insert_stmt_before_use, after finding the insertion point I now do:
+ if (stmt == insert_point)
+gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
+ else
+insert_stmt_after (stmt_to_insert, insert_point);
This is consistent with what is done in other places.
I tested the patch with CPU2006 and bootstrapped and regression tested
for x86-64-none-linux-gnu with no new regressions.
Is this OK for trunk?
Thanks,
Kugan
gcc/testsuite/ChangeLog:
2016-05-28 Kugan Vivekanandarajah
PR middle-end/71269
PR middle-end/71292
* gcc.dg/tree-ssa/pr71269.c: New test.
* gcc.dg/tree-ssa/pr71292.c: New test.
gcc/ChangeLog:
2016-05-28 Kugan Vivekanandarajah
PR middle-end/71269
PR middle-end/71252
* tree-ssa-reassoc.c (insert_stmt_before_use): Use find_insert_point so
that inserted stmt will not dominate stmts that defines its operand.
(rewrite_expr_tree): Add stmt_to_insert before adding the use stmt.
(rewrite_expr_tree_parallel): Likewise.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
index e69de29..4dceaaa 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
@@ -0,0 +1,10 @@
+/* PR middle-end/71269 */
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+int a, b, c;
+void fn2 (int);
+void fn1 ()
+{
+ fn2 (sizeof 0 + c + a + b + b);
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c
index e69de29..1a25d93 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71292.c
@@ -0,0 +1,12 @@
+/* PR middle-end/71292 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+unsigned long a;
+long b, d;
+int c;
+void fn1 ()
+{
+ unsigned long e = a + c;
+ b = d + e + a + 8;
+}
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index c9ed679..bc4b55a 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -1777,16 +1777,6 @@ eliminate_redundant_comparison (enum tree_code opcode,
return false;
}
-/* If the stmt that defines operand has to be inserted, insert it
- before the use. */
-static void
-insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert)
-{
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- gimple_set_uid (stmt_to_insert, gimple_uid (stmt));
- gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
-}
-
/* Transform repeated addition of same values into multiply with
constant. */
@@ -3799,6 +3789,29 @@ find_insert_point (gimple *stmt, tree rhs1, tree rhs2)
return stmt;
}
+/* If the stmt that defines operand has to be inserted, insert it
+ before the use. */
+static void
+insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert)
+{
+ gcc_assert (is_gimple_assign (stmt_to_insert));
+ tree rhs1 = gimple_assign_rhs1 (stmt_to_insert);
+ tree rhs2 = gimple_assign_rhs2 (stmt_to_insert);
+ gimple *insert_point = find_insert_point (stmt, rhs1, rhs2);
+ gimple_stmt_iterator gsi = gsi_for_stmt (insert_point);
+ gimple_set_uid (stmt_to_insert, gimple_uid (insert_point));
+
+ /* If the insert point is not stmt, then insert_point would be
+ the point where operand rhs1 or rhs2 is defined. In this case,
+ stmt_to_insert has to be inserted afterwards. This would
+ only happen when the stmt insertion point is flexible. */
+ if (stmt == insert_point)
+gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
+ else
+insert_stmt_after (stmt_to_insert, insert_point);
+}
+
+
/* Recursively rewrite our linearized statements so that the operators
match those in OPS[OPINDEX], putting the computation in rank
order. Return new lhs. */
@@ -3835,6 +3848,12 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
print_gimple_stmt (dump_file, stmt, 0, 0);
}
+ /* If the stmt that defines operand has to be inserted, insert it
+before the use. */
+ if (oe1->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe1->stmt_to_insert);
+ if (oe2->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe2->stmt_to_insert);
/*
[PATCH2][PR71252] Fix insertion point of stmt_to_insert
Hi Richard,
This fix insertion point of stmt_to_insert based on your comments. In
insert_stmt_before_use , I now use find_insert_point such that we
insert the stmt_to_insert after its operands are defined. This means
that we now have to insert before and insert after in other cases.
I also factored out uses of insert_stmt_before_use.
I tested this with:
./build/gcc/f951 cp2k_single_file.f90 -O3 -ffast-math -march=westmere
I am running bootstrap and regression testing on x86-64-linux gnu. Is
this OK for trunk if the testing is fine ? I will also test with other
test cases from relevant PRs
Thanks,
Kugan
gcc/testsuite/ChangeLog:
2016-05-28 Kugan Vivekanandarajah
* gcc.dg/tree-ssa/pr71269.c: New test.
gcc/ChangeLog:
2016-05-28 Kugan Vivekanandarajah
* tree-ssa-reassoc.c (insert_stmt_before_use): Use find_insert_point so
that inserted stmt will not dominate stmts that defines its operand.
(rewrite_expr_tree): Add stmt_to_insert before adding the use stmt.
(rewrite_expr_tree_parallel): Likewise.
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
index e69de29..4dceaaa 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
@@ -0,0 +1,10 @@
+/* PR middle-end/71269 */
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+int a, b, c;
+void fn2 (int);
+void fn1 ()
+{
+ fn2 (sizeof 0 + c + a + b + b);
+}
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index c9ed679..8a2154f 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -1777,16 +1777,6 @@ eliminate_redundant_comparison (enum tree_code opcode,
return false;
}
-/* If the stmt that defines operand has to be inserted, insert it
- before the use. */
-static void
-insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert)
-{
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- gimple_set_uid (stmt_to_insert, gimple_uid (stmt));
- gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
-}
-
/* Transform repeated addition of same values into multiply with
constant. */
@@ -3799,6 +3789,29 @@ find_insert_point (gimple *stmt, tree rhs1, tree rhs2)
return stmt;
}
+/* If the stmt that defines operand has to be inserted, insert it
+ before the use. */
+static void
+insert_stmt_before_use (gimple *stmt, gimple *stmt_to_insert)
+{
+ gcc_assert (is_gimple_assign (stmt_to_insert));
+ tree rhs1 = gimple_assign_rhs1 (stmt_to_insert);
+ tree rhs2 = gimple_assign_rhs2 (stmt_to_insert);
+ gimple *insert_point = find_insert_point (stmt, rhs1, rhs2);
+ gimple_stmt_iterator gsi = gsi_for_stmt (insert_point);
+ gimple_set_uid (stmt_to_insert, gimple_uid (insert_point));
+
+ /* If the insert point is not stmt, then insert_point would be
+ the point where operand rhs1 or rhs2 is defined. In this case,
+ stmt_to_insert has to be inserted afterwards. This would
+ only happen when the stmt insertion point is flexible. */
+ if (stmt == insert_point)
+gsi_insert_before (&gsi, stmt_to_insert, GSI_NEW_STMT);
+ else
+gsi_insert_after (&gsi, stmt_to_insert, GSI_NEW_STMT);
+}
+
+
/* Recursively rewrite our linearized statements so that the operators
match those in OPS[OPINDEX], putting the computation in rank
order. Return new lhs. */
@@ -3835,6 +3848,12 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
print_gimple_stmt (dump_file, stmt, 0, 0);
}
+ /* If the stmt that defines operand has to be inserted, insert it
+before the use. */
+ if (oe1->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe1->stmt_to_insert);
+ if (oe2->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe2->stmt_to_insert);
/* Even when changed is false, reassociation could have e.g. removed
some redundant operations, so unless we are just swapping the
arguments or unless there is no change at all (then we just
@@ -3843,12 +3862,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
{
gimple *insert_point
= find_insert_point (stmt, oe1->op, oe2->op);
- /* If the stmt that defines operand has to be inserted, insert it
-before the use. */
- if (oe1->stmt_to_insert)
- insert_stmt_before_use (stmt, oe1->stmt_to_insert);
- if (oe2->stmt_to_insert)
- insert_stmt_before_use (stmt, oe2->stmt_to_insert);
lhs = make_ssa_name (TREE_TYPE (lhs));
stmt
= gimple_build_assign (lhs, gimple_assign_rhs_code (stmt),
@@ -3864,12 +3877,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
{
gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op)
== stmt);
- /* If the stmt that defines operand has to be inserted, insert it
-befor
