* [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set
@ 2014-11-20 3:28 Patrick Palka
2014-11-20 6:27 ` Patrick Palka
2014-11-20 13:34 ` Richard Biener
0 siblings, 2 replies; 3+ messages in thread
From: Patrick Palka @ 2014-11-20 3:28 UTC (permalink / raw)
To: gcc-patches; +Cc: Patrick Palka
VRP may simplify a conditional like i <= 5 to i == 5 if it is known that
the lower bound of i's range is 5, e.g. [5, +INF]. But if the upper
bound of i's range is also overflow infinity, i.e. [5, +INF(OVF)] then
this transformation is only valid if -fstrict-overflow is in effect.
Likewise for transforming i > 10 to i != 10 given i's range is
[10, +INF(OVF)] and for transforming i <= 20 to i == 20 given i's range
is [-INF(OVF), 20].
This patch makes this transformation only get performed if strict
overflow rules are in effect and potentially emits a -Wstrict-overflow=3
warning when the transformation takes place.
Bootstrap and regtesting in progress on x86_64-unknown-linux-gnu. Does
the patch look OK if there are no new regressions?
gcc/
* tree-vrp.c (test_for_singularity): New parameter
strict_overflow_p. Set *strict_overflow_p to true if signed
overflow must be undefined for the return value to satisfy the
conditional.
(simplify_cond_using_ranges): Don't perform the simplification
if it violates overflow rules.
gcc/testsuite/
* gcc.dg/no-strict-overflow-8.c: New test.
---
gcc/testsuite/gcc.dg/no-strict-overflow-8.c | 25 +++++++++++++
gcc/tree-vrp.c | 57 +++++++++++++++++++++++++----
2 files changed, 74 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/no-strict-overflow-8.c
diff --git a/gcc/testsuite/gcc.dg/no-strict-overflow-8.c b/gcc/testsuite/gcc.dg/no-strict-overflow-8.c
new file mode 100644
index 0000000..11ef935
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/no-strict-overflow-8.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-fno-strict-overflow -O2 -fdump-tree-optimized" } */
+
+/* We cannot fold i > 0 because p->a - p->b can be larger than INT_MAX
+ and thus i can wrap. Dual of Wstrict-overflow-18.c */
+
+struct c { unsigned int a; unsigned int b; };
+extern void bar (struct c *);
+int
+foo (struct c *p)
+{
+ int i;
+ int sum = 0;
+
+ for (i = 0; i < p->a - p->b; ++i)
+ {
+ if (i > 0)
+ sum += 2;
+ bar (p);
+ }
+ return sum;
+}
+
+/* { dg-final { scan-tree-dump "i_.* > 0" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index bcf4c2b..444af71 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -9117,11 +9117,15 @@ simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
a known value range VR.
If there is one and only one value which will satisfy the
- conditional, then return that value. Else return NULL. */
+ conditional, then return that value. Else return NULL.
+
+ If signed overflow must be undefined for the value to satisfy
+ the conditional, then set *STRICT_OVERFLOW_P to true. */
static tree
test_for_singularity (enum tree_code cond_code, tree op0,
- tree op1, value_range_t *vr)
+ tree op1, value_range_t *vr,
+ bool *strict_overflow_p)
{
tree min = NULL;
tree max = NULL;
@@ -9172,7 +9176,16 @@ test_for_singularity (enum tree_code cond_code, tree op0,
then there is only one value which can satisfy the condition,
return that value. */
if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
- return min;
+ {
+ if ((cond_code == LE_EXPR || cond_code == LT_EXPR)
+ && is_overflow_infinity (vr->max))
+ *strict_overflow_p = true;
+ if ((cond_code == GE_EXPR || cond_code == GT_EXPR)
+ && is_overflow_infinity (vr->min))
+ *strict_overflow_p = true;
+
+ return min;
+ }
}
return NULL;
}
@@ -9252,9 +9265,12 @@ simplify_cond_using_ranges (gcond *stmt)
able to simplify this conditional. */
if (vr->type == VR_RANGE)
{
- tree new_tree = test_for_singularity (cond_code, op0, op1, vr);
+ enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
+ bool sop = false;
+ tree new_tree = test_for_singularity (cond_code, op0, op1, vr, &sop);
- if (new_tree)
+ if (new_tree
+ && (!sop || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0))))
{
if (dump_file)
{
@@ -9275,16 +9291,30 @@ simplify_cond_using_ranges (gcond *stmt)
fprintf (dump_file, "\n");
}
+ if (sop && issue_strict_overflow_warning (wc))
+ {
+ location_t location = input_location;
+ if (gimple_has_location (stmt))
+ location = gimple_location (stmt);
+
+ warning_at (location, OPT_Wstrict_overflow,
+ "assuming signed overflow does not occur when "
+ "simplifying conditional");
+ }
+
return true;
}
/* Try again after inverting the condition. We only deal
with integral types here, so no need to worry about
issues with inverting FP comparisons. */
- cond_code = invert_tree_comparison (cond_code, false);
- new_tree = test_for_singularity (cond_code, op0, op1, vr);
+ sop = false;
+ new_tree = test_for_singularity
+ (invert_tree_comparison (cond_code, false),
+ op0, op1, vr, &sop);
- if (new_tree)
+ if (new_tree
+ && (!sop || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0))))
{
if (dump_file)
{
@@ -9305,6 +9335,17 @@ simplify_cond_using_ranges (gcond *stmt)
fprintf (dump_file, "\n");
}
+ if (sop && issue_strict_overflow_warning (wc))
+ {
+ location_t location = input_location;
+ if (gimple_has_location (stmt))
+ location = gimple_location (stmt);
+
+ warning_at (location, OPT_Wstrict_overflow,
+ "assuming signed overflow does not occur when "
+ "simplifying conditional");
+ }
+
return true;
}
}
--
2.2.0.rc1.23.gf570943
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set
2014-11-20 3:28 [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set Patrick Palka
@ 2014-11-20 6:27 ` Patrick Palka
2014-11-20 13:34 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Patrick Palka @ 2014-11-20 6:27 UTC (permalink / raw)
To: GCC Patches; +Cc: Patrick Palka
On Wed, Nov 19, 2014 at 10:21 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> VRP may simplify a conditional like i <= 5 to i == 5 if it is known that
> the lower bound of i's range is 5, e.g. [5, +INF]. But if the upper
> bound of i's range is also overflow infinity, i.e. [5, +INF(OVF)] then
> this transformation is only valid if -fstrict-overflow is in effect.
> Likewise for transforming i > 10 to i != 10 given i's range is
> [10, +INF(OVF)] and for transforming i <= 20 to i == 20 given i's range
> is [-INF(OVF), 20].
Oops, I meant to say ...and for transforming i >= 20 to i == 20 given
i's range is [-INF(OVF), 20].
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set
2014-11-20 3:28 [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set Patrick Palka
2014-11-20 6:27 ` Patrick Palka
@ 2014-11-20 13:34 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2014-11-20 13:34 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches
On Thu, Nov 20, 2014 at 4:21 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> VRP may simplify a conditional like i <= 5 to i == 5 if it is known that
> the lower bound of i's range is 5, e.g. [5, +INF]. But if the upper
> bound of i's range is also overflow infinity, i.e. [5, +INF(OVF)] then
> this transformation is only valid if -fstrict-overflow is in effect.
> Likewise for transforming i > 10 to i != 10 given i's range is
> [10, +INF(OVF)] and for transforming i <= 20 to i == 20 given i's range
> is [-INF(OVF), 20].
>
> This patch makes this transformation only get performed if strict
> overflow rules are in effect and potentially emits a -Wstrict-overflow=3
> warning when the transformation takes place.
>
> Bootstrap and regtesting in progress on x86_64-unknown-linux-gnu. Does
> the patch look OK if there are no new regressions?
Ok.
Thanks,
Richard.
> gcc/
> * tree-vrp.c (test_for_singularity): New parameter
> strict_overflow_p. Set *strict_overflow_p to true if signed
> overflow must be undefined for the return value to satisfy the
> conditional.
> (simplify_cond_using_ranges): Don't perform the simplification
> if it violates overflow rules.
>
> gcc/testsuite/
> * gcc.dg/no-strict-overflow-8.c: New test.
> ---
> gcc/testsuite/gcc.dg/no-strict-overflow-8.c | 25 +++++++++++++
> gcc/tree-vrp.c | 57 +++++++++++++++++++++++++----
> 2 files changed, 74 insertions(+), 8 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/no-strict-overflow-8.c
>
> diff --git a/gcc/testsuite/gcc.dg/no-strict-overflow-8.c b/gcc/testsuite/gcc.dg/no-strict-overflow-8.c
> new file mode 100644
> index 0000000..11ef935
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/no-strict-overflow-8.c
> @@ -0,0 +1,25 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fno-strict-overflow -O2 -fdump-tree-optimized" } */
> +
> +/* We cannot fold i > 0 because p->a - p->b can be larger than INT_MAX
> + and thus i can wrap. Dual of Wstrict-overflow-18.c */
> +
> +struct c { unsigned int a; unsigned int b; };
> +extern void bar (struct c *);
> +int
> +foo (struct c *p)
> +{
> + int i;
> + int sum = 0;
> +
> + for (i = 0; i < p->a - p->b; ++i)
> + {
> + if (i > 0)
> + sum += 2;
> + bar (p);
> + }
> + return sum;
> +}
> +
> +/* { dg-final { scan-tree-dump "i_.* > 0" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> index bcf4c2b..444af71 100644
> --- a/gcc/tree-vrp.c
> +++ b/gcc/tree-vrp.c
> @@ -9117,11 +9117,15 @@ simplify_bit_ops_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
> a known value range VR.
>
> If there is one and only one value which will satisfy the
> - conditional, then return that value. Else return NULL. */
> + conditional, then return that value. Else return NULL.
> +
> + If signed overflow must be undefined for the value to satisfy
> + the conditional, then set *STRICT_OVERFLOW_P to true. */
>
> static tree
> test_for_singularity (enum tree_code cond_code, tree op0,
> - tree op1, value_range_t *vr)
> + tree op1, value_range_t *vr,
> + bool *strict_overflow_p)
> {
> tree min = NULL;
> tree max = NULL;
> @@ -9172,7 +9176,16 @@ test_for_singularity (enum tree_code cond_code, tree op0,
> then there is only one value which can satisfy the condition,
> return that value. */
> if (operand_equal_p (min, max, 0) && is_gimple_min_invariant (min))
> - return min;
> + {
> + if ((cond_code == LE_EXPR || cond_code == LT_EXPR)
> + && is_overflow_infinity (vr->max))
> + *strict_overflow_p = true;
> + if ((cond_code == GE_EXPR || cond_code == GT_EXPR)
> + && is_overflow_infinity (vr->min))
> + *strict_overflow_p = true;
> +
> + return min;
> + }
> }
> return NULL;
> }
> @@ -9252,9 +9265,12 @@ simplify_cond_using_ranges (gcond *stmt)
> able to simplify this conditional. */
> if (vr->type == VR_RANGE)
> {
> - tree new_tree = test_for_singularity (cond_code, op0, op1, vr);
> + enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_CONDITIONAL;
> + bool sop = false;
> + tree new_tree = test_for_singularity (cond_code, op0, op1, vr, &sop);
>
> - if (new_tree)
> + if (new_tree
> + && (!sop || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0))))
> {
> if (dump_file)
> {
> @@ -9275,16 +9291,30 @@ simplify_cond_using_ranges (gcond *stmt)
> fprintf (dump_file, "\n");
> }
>
> + if (sop && issue_strict_overflow_warning (wc))
> + {
> + location_t location = input_location;
> + if (gimple_has_location (stmt))
> + location = gimple_location (stmt);
> +
> + warning_at (location, OPT_Wstrict_overflow,
> + "assuming signed overflow does not occur when "
> + "simplifying conditional");
> + }
> +
> return true;
> }
>
> /* Try again after inverting the condition. We only deal
> with integral types here, so no need to worry about
> issues with inverting FP comparisons. */
> - cond_code = invert_tree_comparison (cond_code, false);
> - new_tree = test_for_singularity (cond_code, op0, op1, vr);
> + sop = false;
> + new_tree = test_for_singularity
> + (invert_tree_comparison (cond_code, false),
> + op0, op1, vr, &sop);
>
> - if (new_tree)
> + if (new_tree
> + && (!sop || TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (op0))))
> {
> if (dump_file)
> {
> @@ -9305,6 +9335,17 @@ simplify_cond_using_ranges (gcond *stmt)
> fprintf (dump_file, "\n");
> }
>
> + if (sop && issue_strict_overflow_warning (wc))
> + {
> + location_t location = input_location;
> + if (gimple_has_location (stmt))
> + location = gimple_location (stmt);
> +
> + warning_at (location, OPT_Wstrict_overflow,
> + "assuming signed overflow does not occur when "
> + "simplifying conditional");
> + }
> +
> return true;
> }
> }
> --
> 2.2.0.rc1.23.gf570943
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-20 13:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-20 3:28 [PATCH] Disable an unsafe VRP transformation when -fno-strict-overflow is set Patrick Palka
2014-11-20 6:27 ` Patrick Palka
2014-11-20 13:34 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).