diff mbox

[PR71252,PR71269] Fix trunk errors due to stmt_to_insert

Message ID CAELXzTO6r_Tu0kMcfzk73XB5C-qGKWPsOVDeiy3iA47h30Sn7A@mail.gmail.com
State New
Headers show

Commit Message

Kugan Vivekanandarajah May 26, 2016, 4:17 a.m. UTC
Hi,


Sorry for the breakage due to the changes to tree-reassoc. Attached
patch fixes this.

I have tested the patch with spec2006 and cp2k_single_file.f90 from
PR71252 in a x86-640linux-gnu.

./build/gcc/f951 cp2k_single_file.f90 -O3 -ffast-math -march=westmere
works fine.

Tested testcase from PR71269 on gcc2-power8.

In spec2006, I saw couple of errors which I believe are not related:

1.
1464224082.10: /home/kugan.vivekanandarajah/1/usr/local/include/c++/7.0.0/bits/postypes.h:216:5:
note: candidate: template<class _StateT> bool std::operator==(const
std::fpos<_StateT>&, const std::fpos<_StateT>&)
1464224082.10:      operator==(const fpos<_StateT>& __lhs, const
fpos<_StateT>& __rhs)
1464224082.10:      ^~~~~~~~
1464224082.10: /home/kugan.vivekanandarajah/1/usr/local/include/c++/7.0.0/bits/postypes.h:216:5:
note:   template argument deduction/substitution failed:
1464224082.10: mpsinput.cc:75:55: note:
'std::basic_istream<char>::__istream_type {aka
std::basic_istream<char>}' is not derived from 'const
std::fpos<_StateT>'
1464224082.10:           if (m_input.getline(m_buf, sizeof(m_buf)) == 0)
1464224082.10:                                                        ^
1464224082.16: specmake[1]: *** [mpsinput.o] Error 1
1464224082.16: specmake[1]: *** Waiting for unfinished jobs....
1464224084.38: Stop make command: Thu May 26 02:54:44 2016 (1464224084)
1464224084.38: Elapsed time for make command: 00:00:05 (5)
1464224084.38: Error with make 'specmake build': check file
'/home/kugan.vivekanandarajah/CPU2006/benchspec/CPU2006/450.soplex/build/build_base_amd64.0000/make.err'
1464224084.38:   Command returned exit code 2
1464224084.38:   Error with make!
1464224084.39:   Compile for '450.soplex' ended at: Thu May 26
02:54:44 2016 (1464224084)
1464224084.39:   Elapsed compile for '450.soplex': 00:00:05 (5)
1464224084.39: *** Error building 450.soplex


2.
1464224111.69: 0xd99101 thread_through_all_blocks(bool)
1464224111.69:  ../../gcc/gcc/tree-ssa-threadupdate.c:2450
1464224111.70: /home/kugan.vivekanandarajah/1/usr/local/bin/gcc -c -o
SPOOLES/IVL/src/IVL_util.o -DSPEC_CPU -DNDEBUG -ISPOOLES   -static
-static-libgcc -O3 -fno-common       -DSPEC_CPU_LP64
SPOOLES/IVL/src/IVL_util.c
1464224111.71: 0xe202d6 finalize_jump_threads
1464224111.71:  ../../gcc/gcc/tree-vrp.c:10157
1464224111.71: 0xe202d6 execute_vrp
1464224111.71:  ../../gcc/gcc/tree-vrp.c:10311
1464224111.71: 0xe202d6 execute
1464224111.71:  ../../gcc/gcc/tree-vrp.c:10380
1464224111.71: Please submit a full bug report,
1464224111.71: with preprocessed source if appropriate.
1464224111.71: Please include the complete backtrace with any bug report.
1464224111.71: See <http://gcc.gnu.org/bugs.html> for instructions.
1464224111.72: specmake[1]: *** [SPOOLES/Graph/src/Graph_util.o] Error 1
1464224111.72: specmake[1]: *** Waiting for unfinished jobs....
1464224112.43: Stop make command: Thu May 26 02:55:12 2016 (1464224112)
1464224112.43: Elapsed time for make command: 00:00:14 (14)
1464224112.43: Error with make 'specmake build': check file
'/home/kugan.vivekanandarajah/CPU2006/benchspec/CPU2006/454.calculix/build/build_base_amd64.0000/make.err'
1464224112.43:   Command returned exit code 2
1464224112.43:   Error with make!
1464224112.43:   Compile for '454.calculix' ended at: Thu May 26
02:55:12 2016 (1464224112)
1464224112.43:   Elapsed compile for '454.calculix': 00:00:14 (14)
1464224112.43: *** Error building 454.calculix


The issues were:

1. In swap_ops_for_binary_stmt, stmt_to_insert was not swapped. That
was the reason for most of the FAILs. We ended up adding
stmt_to_insert at the wrong place which was not dominating the def.

2. In find_insert_point, I am also making sure that we insert after
the definition of operands of stmt_to_insert.

3. In rewrite_expr_tree_parallel,  build_and_add_sum relies on either
of operand being inserted. If that is not the case, we have to insert
the stmt_to_insert before calling build_and_add_sum.

4. I also moved all the other stmt_to_insert insertion after the use
stmt are created.

Also regression tested on x86-64-linux gnu with no new regressions.

Is this OK for trunk,

Thanks,
Kugan



gcc/ChangeLog:

2016-05-26  Kugan Vivekanandarajah  <kuganv@linaro.org>

    PR middle-end/71269
    PR middle-end/71252
    * tree-ssa-reassoc.c (swap_ops_for_binary_stmt): Handle stmt_to_insert.
    (find_insert_point): If the operands are defined by
    stmt_to_insert, insert after the operand of the stmt_to_insert is defined.
    (rewrite_expr_tree): Add stmt_to_insert after adding the use stmt.
    (reassociate_bb): Likewise.
    (rewrite_expr_tree_parallel): Add stmt_to_insert
    before build_and_add_sum creates new use stmt when the other operand
    to build_and_add_sum dosent have define inst of is a GIMPLE_NOP.


gcc/testsuite/ChangeLog:

2016-05-26  Kugan Vivekanandarajah  <kuganv@linaro.org>

    PR middle-end/71269
    * gcc.dg/tree-ssa/pr71269.c: New test.

Comments

Jakub Jelinek May 26, 2016, 8:18 a.m. UTC | #1
On Thu, May 26, 2016 at 02:17:56PM +1000, Kugan Vivekanandarajah wrote:
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -3767,8 +3767,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>        operand_entry temp = *oe3;
>        oe3->op = oe1->op;
>        oe3->rank = oe1->rank;
> +      oe3->stmt_to_insert = oe1->stmt_to_insert;
>        oe1->op = temp.op;
>        oe1->rank= temp.rank;
> +      oe1->stmt_to_insert = temp.stmt_to_insert;

If you want to swap those 3 fields (what about the others?), can't you write
      std::swap (oe1->op, oe3->op);
      std::swap (oe1->rank, oe3->rank);
      std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert);
instead and drop operand_entry temp = *oe3; ?

>      }
>    else if ((oe1->rank == oe3->rank
>  	    && oe2->rank != oe3->rank)
> @@ -3779,8 +3781,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>        operand_entry temp = *oe2;
>        oe2->op = oe1->op;
>        oe2->rank = oe1->rank;
> +      oe2->stmt_to_insert = oe1->stmt_to_insert;
>        oe1->op = temp.op;
>        oe1->rank = temp.rank;
> +      oe1->stmt_to_insert = temp.stmt_to_insert;
>      }

Similarly.

	Jakub
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
index e69de29..b0c89ea 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
@@ -0,0 +1,9 @@ 
+/* PR middle-end/71269 */
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+int a, b, c;
+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..b9a29a6 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -3767,8 +3767,10 @@  swap_ops_for_binary_stmt (vec<operand_entry *> ops,
       operand_entry temp = *oe3;
       oe3->op = oe1->op;
       oe3->rank = oe1->rank;
+      oe3->stmt_to_insert = oe1->stmt_to_insert;
       oe1->op = temp.op;
       oe1->rank= temp.rank;
+      oe1->stmt_to_insert = temp.stmt_to_insert;
     }
   else if ((oe1->rank == oe3->rank
 	    && oe2->rank != oe3->rank)
@@ -3779,8 +3781,10 @@  swap_ops_for_binary_stmt (vec<operand_entry *> ops,
       operand_entry temp = *oe2;
       oe2->op = oe1->op;
       oe2->rank = oe1->rank;
+      oe2->stmt_to_insert = oe1->stmt_to_insert;
       oe1->op = temp.op;
       oe1->rank = temp.rank;
+      oe1->stmt_to_insert = temp.stmt_to_insert;
     }
 }
 
@@ -3790,6 +3794,42 @@  swap_ops_for_binary_stmt (vec<operand_entry *> ops,
 static inline gimple *
 find_insert_point (gimple *stmt, tree rhs1, tree rhs2)
 {
+  /* If rhs1 is defined by stmt_to_insert, insert after its argument
+     definion stmt.  */
+  if (TREE_CODE (rhs1) == SSA_NAME
+      && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs1))
+      && !gimple_bb (SSA_NAME_DEF_STMT (rhs1)))
+    {
+      gimple *stmt1 = SSA_NAME_DEF_STMT (rhs1);
+      gcc_assert (is_gimple_assign (stmt1));
+      tree rhs11 = gimple_assign_rhs1 (stmt1);
+      tree rhs12 = gimple_assign_rhs2 (stmt1);
+      if (TREE_CODE (rhs11) == SSA_NAME
+	  && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11)))
+	stmt = SSA_NAME_DEF_STMT (rhs11);
+      if (TREE_CODE (rhs12) == SSA_NAME
+	  && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12)))
+	stmt = SSA_NAME_DEF_STMT (rhs12);
+    }
+
+  /* If rhs2 is defined by stmt_to_insert, insert after its argument
+     definion stmt.  */
+  if (TREE_CODE (rhs2) == SSA_NAME
+      && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs2))
+      && !gimple_bb (SSA_NAME_DEF_STMT (rhs2)))
+    {
+      gimple *stmt1 = SSA_NAME_DEF_STMT (rhs2);
+      gcc_assert (is_gimple_assign (stmt1));
+      tree rhs11 = gimple_assign_rhs1 (stmt1);
+      tree rhs12 = gimple_assign_rhs2 (stmt1);
+      if (TREE_CODE (rhs11) == SSA_NAME
+	  && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11)))
+	stmt = SSA_NAME_DEF_STMT (rhs11);
+      if (TREE_CODE (rhs12) == SSA_NAME
+	  && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12)))
+	stmt = SSA_NAME_DEF_STMT (rhs12);
+    }
+
   if (TREE_CODE (rhs1) == SSA_NAME
       && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs1)))
     stmt = SSA_NAME_DEF_STMT (rhs1);
@@ -3845,10 +3885,6 @@  rewrite_expr_tree (gimple *stmt, unsigned int opindex,
 		= 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),
@@ -3866,15 +3902,17 @@  rewrite_expr_tree (gimple *stmt, unsigned int opindex,
 				   == stmt);
 	      /* 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);
 	      gimple_assign_set_rhs1 (stmt, oe1->op);
 	      gimple_assign_set_rhs2 (stmt, oe2->op);
 	      update_stmt (stmt);
 	    }
 
+	  /* If the stmt that defines operand has to be inserted, insert it
+	     before the use after the stmt is inserted.  */
+	  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);
 	  if (rhs1 != oe1->op && rhs1 != oe2->op)
 	    remove_visited_stmt_chain (rhs1);
 
@@ -3893,11 +3931,6 @@  rewrite_expr_tree (gimple *stmt, unsigned int opindex,
   /* Rewrite the next operator.  */
   oe = ops[opindex];
 
-  /* If the stmt that defines operand has to be inserted, insert it
-     before the use.  */
-  if (oe->stmt_to_insert)
-    insert_stmt_before_use (stmt, oe->stmt_to_insert);
-
   /* Recurse on the LHS of the binary operator, which is guaranteed to
      be the non-leaf side.  */
   tree new_rhs1
@@ -3944,6 +3977,10 @@  rewrite_expr_tree (gimple *stmt, unsigned int opindex,
 	  update_stmt (stmt);
 	}
 
+      /* If the stmt that defines operand has to be inserted, insert it
+	 before the use after the use_stmt is inserted.  */
+      if (oe->stmt_to_insert)
+	insert_stmt_before_use (stmt, oe->stmt_to_insert);
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
 	  fprintf (dump_file, " into ");
@@ -4115,24 +4152,41 @@  rewrite_expr_tree_parallel (gassign *stmt, int width,
 	{
 	  /* If the stmt that defines operand has to be inserted, insert it
 	     before the use.  */
-	  if (stmt1)
-	    insert_stmt_before_use (stmts[i], stmt1);
-	  if (stmt2)
-	    insert_stmt_before_use (stmts[i], stmt2);
 	  gimple_assign_set_rhs1 (stmts[i], op1);
 	  gimple_assign_set_rhs2 (stmts[i], op2);
 	  update_stmt (stmts[i]);
 	}
       else
 	{
+	  /* PR71252: stmt_to_insert has to be inserted after use stmt created
+	     by build_and_add_sum. However if the other operand doesnt have define-stmt
+	     or is defined by GIMPLE_NOP, we have to insert it first.  */
+	  if (stmt1
+	      && (TREE_CODE (op2) != SSA_NAME
+		  || !gimple_bb (SSA_NAME_DEF_STMT (op2))
+		  || gimple_nop_p (SSA_NAME_DEF_STMT (op2))))
+	    {
+	      insert_stmt_before_use (stmts[i], stmt1);
+	      stmt1 = NULL;
+	    }
+	  if (stmt2
+	      && (TREE_CODE (op1) != SSA_NAME
+		  || !gimple_bb (SSA_NAME_DEF_STMT (op1))
+		  || gimple_nop_p (SSA_NAME_DEF_STMT (op1))))
+	    {
+	      insert_stmt_before_use (stmts[i], stmt2);
+	      stmt2 = NULL;
+	    }
 	  stmts[i] = build_and_add_sum (TREE_TYPE (last_rhs1), op1, op2, opcode);
-	  /* If the stmt that defines operand has to be inserted, insert it
-	     before new build_and_add stmt after it is created.  */
-	  if (stmt1)
-	    insert_stmt_before_use (stmts[i], stmt1);
-	  if (stmt2)
-	    insert_stmt_before_use (stmts[i], stmt2);
 	}
+
+      /* If the stmt that defines operand has to be inserted, insert it
+	 before new use stmt after it is created.  */
+      if (stmt1)
+	insert_stmt_before_use (stmts[i], stmt1);
+      if (stmt2)
+	insert_stmt_before_use (stmts[i], stmt2);
+      stmt1 = stmt2 = NULL;
       if (dump_file && (dump_flags & TDF_DETAILS))
 	{
 	  fprintf (dump_file, " into ");
@@ -5312,15 +5366,15 @@  reassociate_bb (basic_block bb)
 		{
 		  tree last_op = ops.last ()->op;
 
-		  /* If the stmt that defines operand has to be inserted, insert it
-		     before the use.  */
-		  if (ops.last ()->stmt_to_insert)
-		    insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert);
 		  if (powi_result)
 		    transform_stmt_to_multiply (&gsi, stmt, last_op,
 						powi_result);
 		  else
 		    transform_stmt_to_copy (&gsi, stmt, last_op);
+		  /* If the stmt that defines operand has to be inserted, insert it
+		     before the use.  */
+		  if (ops.last ()->stmt_to_insert)
+		    insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert);
 		}
 	      else
 		{