| Message ID | CAELXzTNFE7xL+Q=8Pq_0HCjNyB2yX_iTWgPzVy2erjQhvQxGiw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On 08/18/2016 08:09 PM, Kugan Vivekanandarajah 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. > > 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. So just to be clear, early VRP simplifies thing in a way that allows CCP (rather than a later VRP) to do the propagation exposing the comparison against an out-of-range constant. Right? > @@ -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)) ^^^^^^^^^ I'm not sure how this could have bootstrapped & regression tested?!? I'm OK with the general direction here, but I think you need to retest ;-) Presumably by the type we get to this test the expression is in canonial form? I'm thinking specifically about a case that may have started off looking like if (x >= y) Where we're able to find a constant for x and that constant is out of the range of what y can hold. jeff
Hi Jeff, On 13/09/16 08:11, Jeff Law wrote: > On 08/18/2016 08:09 PM, Kugan Vivekanandarajah 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. >> >> 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. > So just to be clear, early VRP simplifies thing in a way that allows CCP > (rather than a later VRP) to do the propagation exposing the comparison > against an out-of-range constant. Right? > Yes, thats right. >> @@ -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)) > ^^^^^^^^^ > I'm not sure how this could have bootstrapped & regression tested?!? Sorry, I attached the wrong patch. I resent the correct patch just with this change at : https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01382.html > > I'm OK with the general direction here, but I think you need to retest ;-) > > Presumably by the type we get to this test the expression is in canonial > form? I'm thinking specifically about a case that may have started off > looking like > > if (x >= y) > > Where we're able to find a constant for x and that constant is out of > the range of what y can hold. I agree that it may not be in canonical form all the time, especially when variables becomes constant as in your example. I will resend with this change. Thanks, Kugan > > > jeff > >
From 62152b5499233c2136dc388fd0322bab999f0cb4 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..3c32045 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
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. 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.