diff mbox

[TREE-SSA-CCP] Issue warning when folding condition

Message ID CAELXzTMt3WULoaUgR9jBCN0kYdwMy_5aaqEss1_qdPeEywa_JQ@mail.gmail.com
State New
Headers show

Commit Message

Kugan Vivekanandarajah Aug. 19, 2016, 2:28 a.m. UTC 
On 19 August 2016 at 12:09, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> The testcase pr33738.C for warning fails with early-vrp patch. The

> reason is, with early-vrp ccp2 is folding the comparison that used to

> be folded in simplify_stmt_for_jump_threading. Since early-vrp does

> not perform jump-threading is not optimized there.

>

> Attached patch adds this warning to tree-ssa-ccp.c. We might also run

> into some other similar issues in the future.


Sorry, I attached the wrong patch (with typo). Here is the correct one.

Thanks,
Kugan

>

> Bootstrapped and regression tested on x86_64-linux-gnu with no new regressions.

>

> Is this OK for trunk?

>

> Thanks,

> Kugan

>

> gcc/ChangeLog:

>

> 2016-08-18  Kugan Vivekanandarajah  <kuganv@linaro.org>

>

>     * tree-ssa-ccp.c (ccp_fold_stmt): If the comparison is being folded

>     and the operand on the LHS is being compared against a constant

>     value that is outside of type limit, issue warning.

Comments

Kugan Vivekanandarajah Sept. 12, 2016, 11:52 p.m. UTC | #1 
Hi Richard,


On 19/08/16 18:00, Richard Biener wrote:
> On Fri, 19 Aug 2016, Kugan Vivekanandarajah wrote:

>

>> On 19 August 2016 at 12:09, Kugan Vivekanandarajah

>> <kugan.vivekanandarajah@linaro.org> wrote:

>>> The testcase pr33738.C for warning fails with early-vrp patch. The

>>> reason is, with early-vrp ccp2 is folding the comparison that used to

>>> be folded in simplify_stmt_for_jump_threading. Since early-vrp does

>>> not perform jump-threading is not optimized there.

>>>

>>> Attached patch adds this warning to tree-ssa-ccp.c. We might also run

>>> into some other similar issues in the future.

>>

>> Sorry, I attached the wrong patch (with typo). Here is the correct one.

>

> I think emitting this warning from GIMPLE optimizations is fundamentally

> flawed and the warning should be removed there and put next to

> the cases we alrady handle in c/c-common.c:shorten_compare (or in

> FE specific code).  I see no reason why only VRP or CCP would

> do the simplification for -fstrict-enums enums (thus it seems to be

> missing from the generic comparison folders).

>

But, If I understand this correctly, I think we will not be able to fold 
all the cases we handle in FE. Therefore we will not be able to warn 
there. For very simple cases yes, but not for others.


Thanks,
Kugan

> Richard.

>

>> Thanks,

>> Kugan

>>

>>>

>>> Bootstrapped and regression tested on x86_64-linux-gnu with no new regressions.

>>>

>>> Is this OK for trunk?

>>>

>>> Thanks,

>>> Kugan

>>>

>>> gcc/ChangeLog:

>>>

>>> 2016-08-18  Kugan Vivekanandarajah  <kuganv@linaro.org>

>>>

>>>     * tree-ssa-ccp.c (ccp_fold_stmt): If the comparison is being folded

>>>     and the operand on the LHS is being compared against a constant

>>>     value that is outside of type limit, issue warning.

>>

> k

>
diff mbox

Patch

From 942f0e331f7a92ac46cee8793aac07af74e541e6 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Thu, 18 Aug 2016 20:29:38 +1000
Subject: [PATCH 7/7] Add warn_type_limits to ccp

---
 gcc/tree-ssa-ccp.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index 5d5386e..fce76ce 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -142,6 +142,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "stor-layout.h"
 #include "optabs-query.h"
+#include "diagnostic-core.h"
+#include "intl.h"
 
 
 /* Possible lattice values.  */
@@ -2147,7 +2149,11 @@  ccp_fold_stmt (gimple_stmt_iterator *gsi)
     case GIMPLE_COND:
       {
 	gcond *cond_stmt = as_a <gcond *> (stmt);
+	tree lhs = gimple_cond_lhs (stmt);
+	tree rhs = gimple_cond_rhs (stmt);
 	ccp_prop_value_t val;
+	wide_int min, max, rhs_val;
+	bool warn_limit = false;
 	/* Statement evaluation will handle type mismatches in constants
 	   more gracefully than the final propagation.  This allows us to
 	   fold more conditionals here.  */
@@ -2165,6 +2171,41 @@  ccp_fold_stmt (gimple_stmt_iterator *gsi)
 	    fprintf (dump_file, "\n");
 	  }
 
+	/* If the comparison is being folded and the operand on the LHS
+	   is being compared against a constant value that is outside of
+	   the natural range of LHSs type, then the predicate will
+	   always fold regardless of the value of LHS.  If -Wtype-limits
+	   was specified, emit a warning.  */
+	if (warn_type_limits
+	    && INTEGRAL_TYPE_P (TREE_TYPE (lhs))
+	    && (rhs = get_constant_value (rhs))
+	    && (TREE_CODE (rhs) == INTEGER_CST))
+	  {
+	    rhs_val = rhs;
+	    min = TYPE_MIN_VALUE (TREE_TYPE (lhs));
+	    max = TYPE_MAX_VALUE (TREE_TYPE (lhs));
+	    warn_limit = true;
+	  }
+
+	if (warn_limit
+	    && (wi::cmp (rhs_val, min, TYPE_SIGN (TREE_TYPE (lhs))) == -1
+		|| wi::cmp (max, rhs_val, TYPE_SIGN (TREE_TYPE (lhs))) == -1))
+	  {
+	    location_t location;
+
+	    if (!gimple_has_location (stmt))
+	      location = input_location;
+	    else
+	      location = gimple_location (stmt);
+
+	    warning_at (location, OPT_Wtype_limits,
+			integer_zerop (val.value)
+			? G_("comparison always false "
+			     "due to limited range of data type")
+			: G_("comparison always true "
+			     "due to limited range of data type"));
+	  }
+
 	if (integer_zerop (val.value))
 	  gimple_cond_make_false (cond_stmt);
 	else
-- 
2.7.4