On Tue, Nov 11, 2014 at 8:48 AM, Richard Biener
<[email protected]> wrote:
> On Tue, Nov 11, 2014 at 1:10 PM, Patrick Palka <[email protected]> wrote:
>> This patch is a replacement for the 2nd VRP refactoring patch. It
>> simply teaches VRP to look through widening type conversions when
>> finding suitable edge assertions, e.g.
>>
>> bool p = x != y;
>> int q = (int) p;
>> if (q == 0) // new edge assert: p == 0 and therefore x == y
>
> I think the proper fix is to forward x != y to q == 0 instead of this one.
> That said - the tree-ssa-forwprop.c restriction on only forwarding
> single-uses into conditions is clearly bogus here. I suggest to
> relax it for conversions and compares. Like with
>
> Index: tree-ssa-forwprop.c
> ===================================================================
> --- tree-ssa-forwprop.c (revision 217349)
> +++ tree-ssa-forwprop.c (working copy)
> @@ -476,7 +476,7 @@ forward_propagate_into_comparison_1 (gim
> {
> rhs0 = rhs_to_tree (TREE_TYPE (op1), def_stmt);
> tmp = combine_cond_expr_cond (stmt, code, type,
> - rhs0, op1, !single_use0_p);
> + rhs0, op1, false);
> if (tmp)
> return tmp;
> }
>
>
> Thanks,
> Richard.
That makes sense. Attached is what I have so far. I relaxed the
forwprop restriction in the case of comparing an integer constant with
a comparison or with a conversion from a boolean value. (If I allow
all conversions, not just those from a boolean value, then a couple of
-Wstrict-overflow faillures trigger..) Does the change look sensible?
Should the logic be duplicated for the case when TREE_CODE (op1) ==
SSA_NAME? Thanks for your help so far!
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
new file mode 100644
index 0000000..df5334e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/forwprop-29.c
@@ -0,0 +1,31 @@
+/* { dg-options "-O2" } */
+
+void runtime_error (void) __attribute__ ((noreturn));
+void compiletime_error (void) __attribute__ ((noreturn, error ("")));
+
+static void
+compiletime_check_equals_1 (int *x, int y)
+{
+ int __p = *x != y;
+ if (__builtin_constant_p (__p) && __p)
+ compiletime_error ();
+ if (__p)
+ runtime_error ();
+}
+
+static void
+compiletime_check_equals_2 (int *x, int y)
+{
+ int __p = *x != y;
+ if (__builtin_constant_p (__p) && __p)
+ compiletime_error (); /* { dg-error "call to" } */
+ if (__p)
+ runtime_error ();
+}
+
+void
+foo (int *x)
+{
+ compiletime_check_equals_1 (x, 5);
+ compiletime_check_equals_2 (x, 10);
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
index 251b84e..1ff1820 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr21031.c
@@ -16,5 +16,5 @@ foo (int a)
return 0;
}
-/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1" { xfail *-*-* }
} } */
+/* { dg-final { scan-tree-dump-times "Replaced" 2 "forwprop1" } } */
/* { dg-final { cleanup-tree-dump "forwprop1" } } */
diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index b47f7e2..95f09d1 100644
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -472,11 +472,26 @@ forward_propagate_into_comparison_1 (gimple stmt,
if (TREE_CODE (op0) == SSA_NAME)
{
gimple def_stmt = get_prop_source_stmt (op0, false, &single_use0_p);
+
if (def_stmt && can_propagate_from (def_stmt))
{
+ enum tree_code def_code = gimple_assign_rhs_code (def_stmt);
+ bool invariant_only_p = !single_use0_p;
+
rhs0 = rhs_to_tree (TREE_TYPE (op1), def_stmt);
+
+ /* Always keep the new comparison tree if we are comparing an
+ integer constant with another comparison or with a conversion
+ from a boolean value. */
+ if (TREE_CODE (op1) == INTEGER_CST
+ && ((CONVERT_EXPR_CODE_P (def_code)
+ && TREE_CODE (TREE_TYPE (TREE_OPERAND (rhs0, 0)))
+ == BOOLEAN_TYPE)
+ || TREE_CODE_CLASS (def_code) == tcc_comparison))
+ invariant_only_p = false;
+
tmp = combine_cond_expr_cond (stmt, code, type,
- rhs0, op1, !single_use0_p);
+ rhs0, op1, invariant_only_p);
if (tmp)
return tmp;
}
