This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix PR65851
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Date: Tue, 28 Apr 2015 09:29:18 +0200 (CEST)
- Subject: [PATCH] Fix PR65851
- Authentication-results: sourceware.org; auth=none
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); }