| Message ID | a07dda88-000a-fef5-c2b7-5d155540b013@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Sep 23, 2016 at 12:24 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > Hi, > As Richard pointed out in PR77677, TREE_OVERFLOW is not cleared in IPA-VRP. > There are three places in which we set value_range: > > > 1. When value ranges are obtained from SSA_NAME with get_range_info with > wide_int_to_tree. In this case we will not have TREE_OVERFLOW set. > > 2. When we vrp_meet/vrp_intersect_ranges two ranges. It does int_const_binop > but AFAIK this does not set TREE_OVERFLOW. > > 3. When we create range from constant. This is the problem bit and we need > to clear TREE_OVERFLOW here. > > Attached patch clears the TREE_OVERFLOW in 3rd case. Bootstrap and > regression testing are ongoing. Is this OK if there is no regression. Ok. Though it would be nice to drop it at the source (that is, the point we initialize the IPA-CP lattice and its modifications). Richard. > Thanks, > Kugan > > > gcc/ChangeLog: > > 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR ipa/77677 > * ipa-cp.c (propagate_vr_accross_jump_function):Drop TREE_OVERFLOW > from constant while creating value range. > > gcc/testsuite/ChangeLog: > > 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR ipa/77677 > * gcc.dg/torture/pr77677.c: New test.
Hi Richard, Thanks for the review. On 23/09/16 17:19, Richard Biener wrote: > On Fri, Sep 23, 2016 at 12:24 AM, kugan > <kugan.vivekanandarajah@linaro.org> wrote: >> Hi, >> As Richard pointed out in PR77677, TREE_OVERFLOW is not cleared in IPA-VRP. >> There are three places in which we set value_range: >> >> >> 1. When value ranges are obtained from SSA_NAME with get_range_info with >> wide_int_to_tree. In this case we will not have TREE_OVERFLOW set. >> >> 2. When we vrp_meet/vrp_intersect_ranges two ranges. It does int_const_binop >> but AFAIK this does not set TREE_OVERFLOW. >> >> 3. When we create range from constant. This is the problem bit and we need >> to clear TREE_OVERFLOW here. >> >> Attached patch clears the TREE_OVERFLOW in 3rd case. Bootstrap and >> regression testing are ongoing. Is this OK if there is no regression. > > Ok. Though it would be nice to drop it at the source (that is, the point we > initialize the IPA-CP lattice and its modifications). In ipa_compute_jump_function_for_egde, value_range lattice is not set for constants as this information is already there in IPA_JF_CONSTANT. That is, we initialize only when we get it from get_range_info (SSA_NAMES); others are set to unknown. Though we can set it at this point, it can be inefficient in terms of streaming in/out this data. While propagating we get it from IPA_JF_CONSTANT. Thanks, Kugan > Richard. > >> Thanks, >> Kugan >> >> >> gcc/ChangeLog: >> >> 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR ipa/77677 >> * ipa-cp.c (propagate_vr_accross_jump_function):Drop TREE_OVERFLOW >> from constant while creating value range. >> >> gcc/testsuite/ChangeLog: >> >> 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR ipa/77677 >> * gcc.dg/torture/pr77677.c: New test.
On Fri, Sep 23, 2016 at 10:58 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > Hi Richard, > > Thanks for the review. > > On 23/09/16 17:19, Richard Biener wrote: >> >> On Fri, Sep 23, 2016 at 12:24 AM, kugan >> <kugan.vivekanandarajah@linaro.org> wrote: >>> >>> Hi, >>> As Richard pointed out in PR77677, TREE_OVERFLOW is not cleared in >>> IPA-VRP. >>> There are three places in which we set value_range: >>> >>> >>> 1. When value ranges are obtained from SSA_NAME with get_range_info with >>> wide_int_to_tree. In this case we will not have TREE_OVERFLOW set. >>> >>> 2. When we vrp_meet/vrp_intersect_ranges two ranges. It does >>> int_const_binop >>> but AFAIK this does not set TREE_OVERFLOW. >>> >>> 3. When we create range from constant. This is the problem bit and we >>> need >>> to clear TREE_OVERFLOW here. >>> >>> Attached patch clears the TREE_OVERFLOW in 3rd case. Bootstrap and >>> regression testing are ongoing. Is this OK if there is no regression. >> >> >> Ok. Though it would be nice to drop it at the source (that is, the point >> we >> initialize the IPA-CP lattice and its modifications). > > > In ipa_compute_jump_function_for_egde, value_range lattice is not set for > constants as this information is already there in IPA_JF_CONSTANT. That is, > we initialize only when we get it from get_range_info (SSA_NAMES); others > are set to unknown. Though we can set it at this point, it can be > inefficient in terms of streaming in/out this data. While propagating we get > it from IPA_JF_CONSTANT. Yes, I meant we should avoid TREE_OVERFLOW on IPA_JF_CONSTANT in the first place. Richard. > > Thanks, > Kugan > > >> Richard. >> >>> Thanks, >>> Kugan >>> >>> >>> gcc/ChangeLog: >>> >>> 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> PR ipa/77677 >>> * ipa-cp.c (propagate_vr_accross_jump_function):Drop >>> TREE_OVERFLOW >>> from constant while creating value range. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2016-09-23 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> PR ipa/77677 >>> * gcc.dg/torture/pr77677.c: New test.
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index cb60f1e..f735ef7 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -2065,6 +2065,8 @@ propagate_vr_accross_jump_function (cgraph_edge *cs, tree val = ipa_get_jf_constant (jfunc); if (TREE_CODE (val) == INTEGER_CST) { + if (TREE_OVERFLOW_P (val)) + val = drop_tree_overflow (val); jfunc->vr_known = true; jfunc->m_vr.type = VR_RANGE; jfunc->m_vr.min = val; diff --git a/gcc/testsuite/gcc.dg/torture/pr77677.c b/gcc/testsuite/gcc.dg/torture/pr77677.c index e69de29..af3f0b0 100644 --- a/gcc/testsuite/gcc.dg/torture/pr77677.c +++ b/gcc/testsuite/gcc.dg/torture/pr77677.c @@ -0,0 +1,18 @@ +/* PR ipa/77677 */ +/* { dg-do compile } */ + +int a, b; + +static void fn1 (short p1) +{ + a = -p1; + if (a || b) + __builtin_printf ("%d\n", b); +} + +int main () +{ + int c[] = { 40000 }; + fn1 (c[0]); + return 0; +}