[PATCH] Fix PR65851

Richard Biener rguenther@suse.de
Tue Apr 28 07:49:00 GMT 2015


This fixes PR65851 and a few missed optimizations in CCP that can cause
similar issues.  This also fixes set_lattice_value to pass new_val
by reference so that in case we drop to VARYING the caller can see that.
It also uses operand_equal_p instead of simple_cst_equal because the
latter doesn't even handle all tcc_constant trees...

Finally it simplifies set_lattice_value to use the meet operator
in case we go from CONSTANT to (non-copy) CONSTANT.  This avoids
dropping to VARYING for some common cases.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2015-04-28  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/65851
	* tree-ssa-ccp.c (set_lattice_value): Perform a meet when
	changing CONSTANT to CONSTANT non-copy.  Get new_val by reference.
	(ccp_lattice_meet): Remove stray argument.  Use operand_equal_p
	rather than simple_cst_equal as the latter doesn't handle COMPLEX_CST.
	(ccp_visit_phi_node): Adjust.
	(evaluate_stmt): For simplifications to SSA names return its
	lattice value if that isn't VARYING.  Return immediately when
	simplified to a constant.
	(visit_assignment): Adjust.
	(ccp_visit_stmt): Likewise.

	* g++.dg/torture/pr65851.C: New testcase.

Index: gcc/tree-ssa-ccp.c
===================================================================
*** gcc/tree-ssa-ccp.c	(revision 222453)
--- gcc/tree-ssa-ccp.c	(working copy)
*************** static unsigned n_const_val;
*** 208,213 ****
--- 208,214 ----
  
  static void canonicalize_value (ccp_prop_value_t *);
  static bool ccp_fold_stmt (gimple_stmt_iterator *);
+ static void ccp_lattice_meet (ccp_prop_value_t *, ccp_prop_value_t *);
  
  /* Dump constant propagation value VAL to file OUTF prefixed by PREFIX.  */
  
*************** valid_lattice_transition (ccp_prop_value
*** 512,565 ****
     value is different from VAR's previous value.  */
  
  static bool
! set_lattice_value (tree var, ccp_prop_value_t new_val)
  {
    /* We can deal with old UNINITIALIZED values just fine here.  */
    ccp_prop_value_t *old_val = &const_val[SSA_NAME_VERSION (var)];
  
!   canonicalize_value (&new_val);
  
    /* We have to be careful to not go up the bitwise lattice
!      represented by the mask.
!      ???  This doesn't seem to be the best place to enforce this.  */
!   if (new_val.lattice_val == CONSTANT
        && old_val->lattice_val == CONSTANT
!       && TREE_CODE (new_val.value) == INTEGER_CST
!       && TREE_CODE (old_val->value) == INTEGER_CST)
!     {
!       widest_int diff = (wi::to_widest (new_val.value)
! 			 ^ wi::to_widest (old_val->value));
!       new_val.mask = new_val.mask | old_val->mask | diff;
!     }
  
!   gcc_checking_assert (valid_lattice_transition (*old_val, new_val));
  
    /* If *OLD_VAL and NEW_VAL are the same, return false to inform the
       caller that this was a non-transition.  */
!   if (old_val->lattice_val != new_val.lattice_val
!       || (new_val.lattice_val == CONSTANT
! 	  && (TREE_CODE (new_val.value) != TREE_CODE (old_val->value)
! 	      || (TREE_CODE (new_val.value) == INTEGER_CST
! 		  && (new_val.mask != old_val->mask
  		      || (wi::bit_and_not (wi::to_widest (old_val->value),
! 					   new_val.mask)
! 			  != wi::bit_and_not (wi::to_widest (new_val.value),
! 					      new_val.mask))))
! 	      || (TREE_CODE (new_val.value) != INTEGER_CST
! 		  && !operand_equal_p (new_val.value, old_val->value, 0)))))
      {
        /* ???  We would like to delay creation of INTEGER_CSTs from
  	 partially constants here.  */
  
        if (dump_file && (dump_flags & TDF_DETAILS))
  	{
! 	  dump_lattice_value (dump_file, "Lattice value changed to ", new_val);
  	  fprintf (dump_file, ".  Adding SSA edges to worklist.\n");
  	}
  
!       *old_val = new_val;
  
!       gcc_assert (new_val.lattice_val != UNINITIALIZED);
        return true;
      }
  
--- 513,563 ----
     value is different from VAR's previous value.  */
  
  static bool
! set_lattice_value (tree var, ccp_prop_value_t *new_val)
  {
    /* We can deal with old UNINITIALIZED values just fine here.  */
    ccp_prop_value_t *old_val = &const_val[SSA_NAME_VERSION (var)];
  
!   canonicalize_value (new_val);
  
    /* We have to be careful to not go up the bitwise lattice
!      represented by the mask.  Instead of dropping to VARYING
!      use the meet operator to retain a conservative value.
!      Missed optimizations like PR65851 makes this necessary.
!      It also ensures we converge to a stable lattice solution.  */
!   if (new_val->lattice_val == CONSTANT
        && old_val->lattice_val == CONSTANT
!       && TREE_CODE (new_val->value) != SSA_NAME)
!     ccp_lattice_meet (new_val, old_val);
  
!   gcc_checking_assert (valid_lattice_transition (*old_val, *new_val));
  
    /* If *OLD_VAL and NEW_VAL are the same, return false to inform the
       caller that this was a non-transition.  */
!   if (old_val->lattice_val != new_val->lattice_val
!       || (new_val->lattice_val == CONSTANT
! 	  && (TREE_CODE (new_val->value) != TREE_CODE (old_val->value)
! 	      || (TREE_CODE (new_val->value) == INTEGER_CST
! 		  && (new_val->mask != old_val->mask
  		      || (wi::bit_and_not (wi::to_widest (old_val->value),
! 					   new_val->mask)
! 			  != wi::bit_and_not (wi::to_widest (new_val->value),
! 					      new_val->mask))))
! 	      || (TREE_CODE (new_val->value) != INTEGER_CST
! 		  && !operand_equal_p (new_val->value, old_val->value, 0)))))
      {
        /* ???  We would like to delay creation of INTEGER_CSTs from
  	 partially constants here.  */
  
        if (dump_file && (dump_flags & TDF_DETAILS))
  	{
! 	  dump_lattice_value (dump_file, "Lattice value changed to ", *new_val);
  	  fprintf (dump_file, ".  Adding SSA edges to worklist.\n");
  	}
  
!       *old_val = *new_val;
  
!       gcc_assert (new_val->lattice_val != UNINITIALIZED);
        return true;
      }
  
*************** ccp_finalize (void)
*** 991,998 ****
     */
  
  static void
! ccp_lattice_meet (basic_block where,
! 		  ccp_prop_value_t *val1, ccp_prop_value_t *val2)
  {
    if (val1->lattice_val == UNDEFINED
        /* For UNDEFINED M SSA we can't always SSA because its definition
--- 989,995 ----
     */
  
  static void
! ccp_lattice_meet (ccp_prop_value_t *val1, ccp_prop_value_t *val2)
  {
    if (val1->lattice_val == UNDEFINED
        /* For UNDEFINED M SSA we can't always SSA because its definition
*************** ccp_lattice_meet (basic_block where,
*** 1042,1048 ****
      }
    else if (val1->lattice_val == CONSTANT
  	   && val2->lattice_val == CONSTANT
! 	   && simple_cst_equal (val1->value, val2->value) == 1)
      {
        /* Ci M Cj = Ci		if (i == j)
  	 Ci M Cj = VARYING	if (i != j)
--- 1039,1045 ----
      }
    else if (val1->lattice_val == CONSTANT
  	   && val2->lattice_val == CONSTANT
! 	   && operand_equal_p (val1->value, val2->value, 0))
      {
        /* Ci M Cj = Ci		if (i == j)
  	 Ci M Cj = VARYING	if (i != j)
*************** ccp_lattice_meet (basic_block where,
*** 1061,1067 ****
  	*val1 = get_value_for_expr (val1->value, true);
        if (TREE_CODE (val2->value) == ADDR_EXPR)
  	tem = get_value_for_expr (val2->value, true);
!       ccp_lattice_meet (where, val1, &tem);
      }
    else
      {
--- 1058,1064 ----
  	*val1 = get_value_for_expr (val1->value, true);
        if (TREE_CODE (val2->value) == ADDR_EXPR)
  	tem = get_value_for_expr (val2->value, true);
!       ccp_lattice_meet (val1, &tem);
      }
    else
      {
*************** ccp_visit_phi_node (gphi *phi)
*** 1122,1128 ****
  	      first = false;
  	    }
  	  else
! 	    ccp_lattice_meet (gimple_bb (phi), &new_val, &arg_val);
  
  	  if (dump_file && (dump_flags & TDF_DETAILS))
  	    {
--- 1119,1125 ----
  	      first = false;
  	    }
  	  else
! 	    ccp_lattice_meet (&new_val, &arg_val);
  
  	  if (dump_file && (dump_flags & TDF_DETAILS))
  	    {
*************** ccp_visit_phi_node (gphi *phi)
*** 1144,1150 ****
      }
  
    /* Make the transition to the new value.  */
!   if (set_lattice_value (gimple_phi_result (phi), new_val))
      {
        if (new_val.lattice_val == VARYING)
  	return SSA_PROP_VARYING;
--- 1141,1147 ----
      }
  
    /* Make the transition to the new value.  */
!   if (set_lattice_value (gimple_phi_result (phi), &new_val))
      {
        if (new_val.lattice_val == VARYING)
  	return SSA_PROP_VARYING;
*************** evaluate_stmt (gimple stmt)
*** 1745,1750 ****
--- 1742,1756 ----
      {
        fold_defer_overflow_warnings ();
        simplified = ccp_fold (stmt);
+       if (simplified && TREE_CODE (simplified) == SSA_NAME)
+ 	{
+ 	  val = *get_value (simplified);
+ 	  if (val.lattice_val != VARYING)
+ 	    {
+ 	      fold_undefer_overflow_warnings (true, stmt, 0);
+ 	      return val;
+ 	    }
+ 	}
        is_constant = simplified && is_gimple_min_invariant (simplified);
        fold_undefer_overflow_warnings (is_constant, stmt, 0);
        if (is_constant)
*************** evaluate_stmt (gimple stmt)
*** 1753,1758 ****
--- 1759,1765 ----
  	  val.lattice_val = CONSTANT;
  	  val.value = simplified;
  	  val.mask = 0;
+ 	  return val;
  	}
      }
    /* If the statement is likely to have a VARYING result, then do not
*************** visit_assignment (gimple stmt, tree *out
*** 2293,2299 ****
  
        /* If STMT is an assignment to an SSA_NAME, we only have one
  	 value to set.  */
!       if (set_lattice_value (lhs, val))
  	{
  	  *output_p = lhs;
  	  if (val.lattice_val == VARYING)
--- 2300,2306 ----
  
        /* If STMT is an assignment to an SSA_NAME, we only have one
  	 value to set.  */
!       if (set_lattice_value (lhs, &val))
  	{
  	  *output_p = lhs;
  	  if (val.lattice_val == VARYING)
*************** ccp_visit_stmt (gimple stmt, edge *taken
*** 2391,2400 ****
       SSA_NAMEs represent unknown modifications to their outputs.
       Mark them VARYING.  */
    FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
!     {
!       ccp_prop_value_t v = { VARYING, NULL_TREE, -1 };
!       set_lattice_value (def, v);
!     }
  
    return SSA_PROP_VARYING;
  }
--- 2398,2404 ----
       SSA_NAMEs represent unknown modifications to their outputs.
       Mark them VARYING.  */
    FOR_EACH_SSA_TREE_OPERAND (def, stmt, iter, SSA_OP_ALL_DEFS)
!     set_value_varying (def);
  
    return SSA_PROP_VARYING;
  }
Index: gcc/testsuite/g++.dg/torture/pr65851.C
===================================================================
*** gcc/testsuite/g++.dg/torture/pr65851.C	(revision 0)
--- gcc/testsuite/g++.dg/torture/pr65851.C	(working copy)
***************
*** 0 ****
--- 1,37 ----
+ // { dg-do compile }
+ class A {
+   virtual unsigned long write(const char *, unsigned long);
+ };
+ char *a;
+ int b;
+ bool c;
+ char e[16];
+ class B {
+ public:
+   void push_range(const char *);
+ };
+ class C : A {
+   B m_string;
+ 
+ public:
+   unsigned long write(const char *p1, unsigned long p2) {
+     m_string.push_range(p1 + p2);
+   }
+ };
+ char *write_signed_decimal_backward(bool) {
+   char *d = 0;
+   if (b) {
+     if (c)
+       --a;
+     d = a;
+   }
+   return d;
+ }
+ 
+ template <typename TextOutputStreamType>
+ void ostream_write(TextOutputStreamType &p1) {
+   char *f = write_signed_decimal_backward(false);
+   p1.write(f, e - f);
+ }
+ 
+ void operator<<(C p1, int) { ostream_write(p1); }



More information about the Gcc-patches mailing list