Reassoc zero_one_operation fixes (PR tree-optimization/78726)

Hi!

The following testcases show two bugs in zero_one_operation (used during
undistribute optimization) and functions it calls.
The first one (wrong-code) is fixed by the
+  tree orig_def = *def;
and
-  if (stmts_to_fix.length () > 0)
+  if (stmts_to_fix.length () > 0 || *def == orig_def)
changes in zero_one_operation - e.g. in the testcase we have before reassoc
  _6 = _5 * a_11;
  _7 = _5 * _6;
and call zero_one_operation for _7 with op a_11.  The last stmt isn't pushed
into stmts_to_fix, the *def stmt is handled specially in
make_new_ssa_for_all_defs.  And the previous one, while it is pushed, it is
popped from there again, because the stmt is removed and instead _6 use
is replaced with _5.  That changes what value _7 holds, so we want to use
a new SSA_NAME for it (or deal with debug stmts and reset flow sensitive
info etc.).

The second one is wrong-debug, make_new_ssa_for_def replaces the uses of old
lhs with the new lhs (which is fine in the code uses, but for debug info
because we zeroed out a_11 we want to say that previous uses of _7 are now
the _7's replacement * a_11.  Which is what the rest of the patch does
and there is a guality testcase that verifies it (previously it would fail).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-12-08  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/78726
	* tree-ssa-reassoc.c (make_new_ssa_for_def): Add OPCODE and OP
	argument.  For lhs uses in debug stmts, don't replace lhs with
	new_lhs, but with a debug temp set to new_lhs opcode op.
	(make_new_ssa_for_all_defs): Add OPCODE argument, pass OPCODE and
	OP down to make_new_ssa_for_def.
	(zero_one_operation): Call make_new_ssa_for_all_defs even when
	stmts_to_fix is empty, if *def has not changed yet.  Pass
	OPCODE to make_new_ssa_for_all_defs.

	* gcc.c-torture/execute/pr78726.c: New test.
	* gcc.dg/guality/pr78726.c: New test.


	Jakub

On Thu, 8 Dec 2016, Jakub Jelinek wrote:

> Hi!
> 
> The following testcases show two bugs in zero_one_operation (used during
> undistribute optimization) and functions it calls.
> The first one (wrong-code) is fixed by the
> +  tree orig_def = *def;
> and
> -  if (stmts_to_fix.length () > 0)
> +  if (stmts_to_fix.length () > 0 || *def == orig_def)
> changes in zero_one_operation - e.g. in the testcase we have before reassoc
>   _6 = _5 * a_11;
>   _7 = _5 * _6;
> and call zero_one_operation for _7 with op a_11.  The last stmt isn't pushed
> into stmts_to_fix, the *def stmt is handled specially in
> make_new_ssa_for_all_defs.  And the previous one, while it is pushed, it is
> popped from there again, because the stmt is removed and instead _6 use
> is replaced with _5.  That changes what value _7 holds, so we want to use
> a new SSA_NAME for it (or deal with debug stmts and reset flow sensitive
> info etc.).
> 
> The second one is wrong-debug, make_new_ssa_for_def replaces the uses of old
> lhs with the new lhs (which is fine in the code uses, but for debug info
> because we zeroed out a_11 we want to say that previous uses of _7 are now
> the _7's replacement * a_11.  Which is what the rest of the patch does
> and there is a guality testcase that verifies it (previously it would fail).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2016-12-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/78726
> 	* tree-ssa-reassoc.c (make_new_ssa_for_def): Add OPCODE and OP
> 	argument.  For lhs uses in debug stmts, don't replace lhs with
> 	new_lhs, but with a debug temp set to new_lhs opcode op.
> 	(make_new_ssa_for_all_defs): Add OPCODE argument, pass OPCODE and
> 	OP down to make_new_ssa_for_def.
> 	(zero_one_operation): Call make_new_ssa_for_all_defs even when
> 	stmts_to_fix is empty, if *def has not changed yet.  Pass
> 	OPCODE to make_new_ssa_for_all_defs.
> 
> 	* gcc.c-torture/execute/pr78726.c: New test.
> 	* gcc.dg/guality/pr78726.c: New test.
> 
> --- gcc/tree-ssa-reassoc.c.jj	2016-11-09 16:34:58.000000000 +0100
> +++ gcc/tree-ssa-reassoc.c	2016-12-08 15:53:13.768894146 +0100
> @@ -1153,12 +1153,12 @@ decrement_power (gimple *stmt)
>     SSA.  Also return the new SSA.  */
>  
>  static tree
> -make_new_ssa_for_def (gimple *stmt)
> +make_new_ssa_for_def (gimple *stmt, enum tree_code opcode, tree op)
>  {
>    gimple *use_stmt;
>    use_operand_p use;
>    imm_use_iterator iter;
> -  tree new_lhs;
> +  tree new_lhs, new_debug_lhs = NULL_TREE;
>    tree lhs = gimple_get_lhs (stmt);
>  
>    new_lhs = make_ssa_name (TREE_TYPE (lhs));
> @@ -1167,8 +1167,28 @@ make_new_ssa_for_def (gimple *stmt)
>    /* Also need to update GIMPLE_DEBUGs.  */
>    FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
>      {
> +      tree repl = new_lhs;
> +      if (is_gimple_debug (use_stmt))
> +	{
> +	  if (new_debug_lhs == NULL_TREE)
> +	    {
> +	      new_debug_lhs = make_node (DEBUG_EXPR_DECL);
> +	      gdebug *def_temp
> +		= gimple_build_debug_bind (new_debug_lhs,
> +					   build2 (opcode, TREE_TYPE (lhs),
> +						   new_lhs, op),
> +					   stmt);
> +	      DECL_ARTIFICIAL (new_debug_lhs) = 1;
> +	      TREE_TYPE (new_debug_lhs) = TREE_TYPE (lhs);
> +	      SET_DECL_MODE (new_debug_lhs, TYPE_MODE (TREE_TYPE (lhs)));
> +	      gimple_set_uid (def_temp, gimple_uid (stmt));
> +	      gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> +	      gsi_insert_after (&gsi, def_temp, GSI_SAME_STMT);
> +	    }
> +	  repl = new_debug_lhs;
> +	}
>        FOR_EACH_IMM_USE_ON_STMT (use, iter)
> -	SET_USE (use, new_lhs);
> +	SET_USE (use, repl);
>        update_stmt (use_stmt);
>      }
>    return new_lhs;
> @@ -1179,7 +1199,7 @@ make_new_ssa_for_def (gimple *stmt)
>     if *DEF is not OP.  */
>  
>  static void
> -make_new_ssa_for_all_defs (tree *def, tree op,
> +make_new_ssa_for_all_defs (tree *def, enum tree_code opcode, tree op,
>  			   vec<gimple *> &stmts_to_fix)
>  {
>    unsigned i;
> @@ -1189,10 +1209,10 @@ make_new_ssa_for_all_defs (tree *def, tr
>        && TREE_CODE (*def) == SSA_NAME
>        && (stmt = SSA_NAME_DEF_STMT (*def))
>        && gimple_code (stmt) != GIMPLE_NOP)
> -    *def = make_new_ssa_for_def (stmt);
> +    *def = make_new_ssa_for_def (stmt, opcode, op);
>  
>    FOR_EACH_VEC_ELT (stmts_to_fix, i, stmt)
> -    make_new_ssa_for_def (stmt);
> +    make_new_ssa_for_def (stmt, opcode, op);
>  }
>  
>  /* Find the single immediate use of STMT's LHS, and replace it
> @@ -1232,6 +1252,7 @@ propagate_op_to_single_use (tree op, gim
>  static void
>  zero_one_operation (tree *def, enum tree_code opcode, tree op)
>  {
> +  tree orig_def = *def;
>    gimple *stmt = SSA_NAME_DEF_STMT (*def);
>    /* PR72835 - Record the stmt chain that has to be updated such that
>       we dont use the same LHS when the values computed are different.  */
> @@ -1335,8 +1356,8 @@ zero_one_operation (tree *def, enum tree
>      }
>    while (1);
>  
> -  if (stmts_to_fix.length () > 0)
> -    make_new_ssa_for_all_defs (def, op, stmts_to_fix);
> +  if (stmts_to_fix.length () > 0 || *def == orig_def)
> +    make_new_ssa_for_all_defs (def, opcode, op, stmts_to_fix);
>  }
>  
>  /* Returns true if statement S1 dominates statement S2.  Like
> --- gcc/testsuite/gcc.c-torture/execute/pr78726.c.jj	2016-12-08 15:55:08.847434702 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr78726.c	2016-12-08 15:54:46.000000000 +0100
> @@ -0,0 +1,23 @@
> +/* PR tree-optimization/78726 */
> +
> +unsigned char b = 36, c = 173;
> +unsigned int d;
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> +  unsigned a = ~b;
> +  d = a * c * c + 1023094746U * a;
> +}
> +
> +int
> +main ()
> +{
> +  if (__SIZEOF_INT__ != 4 || __CHAR_BIT__ != 8)
> +    return 0;
> +  asm volatile ("" : : "g" (&b), "g" (&c) : "memory");
> +  foo ();
> +  if (d != 799092689U)
> +    __builtin_abort ();
> +  return 0;
> +}
> --- gcc/testsuite/gcc.dg/guality/pr78726.c.jj	2016-12-08 15:57:07.779926381 +0100
> +++ gcc/testsuite/gcc.dg/guality/pr78726.c	2016-12-08 16:02:57.216494771 +0100
> @@ -0,0 +1,30 @@
> +/* PR tree-optimization/78726 */
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +#include "../nop.h"
> +
> +unsigned char b = 36, c = 173;
> +unsigned int d;
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> +  unsigned a = ~b;
> +  unsigned d1 = a * c;		/* { dg-final { gdb-test 21 "d1" "~36U * 173" } } */
> +  unsigned d2 = d1 * c;		/* { dg-final { gdb-test 21 "d2" "~36U * 173 * 173" } } */
> +  unsigned d3 = 1023094746 * a;	/* { dg-final { gdb-test 21 "d3" "~36U * 1023094746" } } */
> +  d = d2 + d3;
> +  unsigned d4 = d1 * 2;     	/* { dg-final { gdb-test 21 "d4" "~36U * 173 * 2" } } */
> +  unsigned d5 = d2 * 2;		/* { dg-final { gdb-test 21 "d5" "~36U * 173 * 173 * 2" } } */
> +  unsigned d6 = d3 * 2;		/* { dg-final { gdb-test 21 "d6" "~36U * 1023094746 * 2" } } */
> +  asm (NOP : : : "memory");
> +}
> +
> +int
> +main ()
> +{
> +  asm volatile ("" : : "g" (&b), "g" (&c) : "memory");
> +  foo ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>