* ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
@ 2016-09-22 22:52 kugan
2016-09-23 7:52 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: kugan @ 2016-09-22 22:52 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, Martin Jambor, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 995 bytes --]
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.
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.
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 894 bytes --]
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;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
2016-09-22 22:52 ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu kugan
@ 2016-09-23 7:52 ` Richard Biener
2016-09-23 9:15 ` kugan
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2016-09-23 7:52 UTC (permalink / raw)
To: kugan; +Cc: gcc-patches, Martin Jambor, Jan Hubicka
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
2016-09-23 7:52 ` Richard Biener
@ 2016-09-23 9:15 ` kugan
2016-09-23 9:37 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: kugan @ 2016-09-23 9:15 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Martin Jambor, Jan Hubicka
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
2016-09-23 9:15 ` kugan
@ 2016-09-23 9:37 ` Richard Biener
2016-09-24 7:11 ` kugan
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2016-09-23 9:37 UTC (permalink / raw)
To: kugan; +Cc: gcc-patches, Martin Jambor, Jan Hubicka
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
2016-09-23 9:37 ` Richard Biener
@ 2016-09-24 7:11 ` kugan
2016-09-26 7:45 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: kugan @ 2016-09-24 7:11 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Martin Jambor, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
Hi Richard,
There is also one more issue as reported by Pat Haugen. That is, in
converting value_range of ssa (which we get from get_range_info) to
argument type, my implementation is too simplistic and wrong at times. I
can check TYPE_UNSIGNED here but that would be pessimistic.
tree-vrp already has logic to handle this so the attached patch exports
this and uses it.
This also would be useful when we pass an argument of the function to
anther function call with unary operation. I will send a separate patch
for that later if this is OK.
Bootstrapped and regression tested on x86_64-linux-gnu with no new
regressions. Is this OK for trunk?
Thanks,
Kugan
gcc/ChangeLog:
2016-09-24 Kugan Vivekanandarajah <kuganv@linaro.org>
PR ipa/77677
* ipa-prop.c (ipa_compute_jump_functions_for_edge): Use
extract_range_from_unary_expr to convert value_range.
* tree-vrp.c (extract_range_from_unary_expr_1): Rename to.
(extract_range_from_unary_expr): This.
* tree-vrp.h (extract_range_from_unary_expr): Declare.
gcc/testsuite/ChangeLog:
2016-09-24 Kugan Vivekanandarajah <kuganv@linaro.org>
PR ipa/77677
* gcc.dg/torture/pr77677-2.c: New test.
[-- Attachment #2: p.txt --]
[-- Type: text/plain, Size: 3870 bytes --]
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index feecd23..f1f641b 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1703,13 +1703,22 @@ ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
if (TREE_CODE (arg) == SSA_NAME
&& param_type
&& (type = get_range_info (arg, &min, &max))
- && (type == VR_RANGE || type == VR_ANTI_RANGE)
- && (min.get_precision () <= TYPE_PRECISION (param_type)))
+ && (type == VR_RANGE || type == VR_ANTI_RANGE))
{
- jfunc->vr_known = true;
- jfunc->m_vr.type = type;
- jfunc->m_vr.min = wide_int_to_tree (param_type, min);
- jfunc->m_vr.max = wide_int_to_tree (param_type, max);
+ value_range vr;
+
+ vr.type = type;
+ vr.min = wide_int_to_tree (TREE_TYPE (arg), min);
+ vr.max = wide_int_to_tree (TREE_TYPE (arg), max);
+ vr.equiv = NULL;
+ extract_range_from_unary_expr_range (&jfunc->m_vr,
+ NOP_EXPR,
+ param_type,
+ &vr, TREE_TYPE (arg));
+ if (jfunc->m_vr.type == VR_RANGE || jfunc->m_vr.type == VR_ANTI_RANGE)
+ jfunc->vr_known = true;
+ else
+ jfunc->vr_known = false;
}
else
gcc_assert (!jfunc->vr_known);
diff --git a/gcc/testsuite/gcc.dg/torture/pr77677-2.c b/gcc/testsuite/gcc.dg/torture/pr77677-2.c
index e69de29..661e6a3 100644
--- a/gcc/testsuite/gcc.dg/torture/pr77677-2.c
+++ b/gcc/testsuite/gcc.dg/torture/pr77677-2.c
@@ -0,0 +1,16 @@
+/* PR ipa/77677 */
+/* { dg-do compile } */
+
+enum machine_mode { MAX_MACHINE_MODE };
+
+struct {
+ int mode : 8;
+} a;
+int b;
+
+static int fn1();
+
+void fn2() { fn1(a, a.mode); }
+
+int fn1(a, mode) enum machine_mode mode;
+{ int c = b = c; }
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 3c75a0d..39a33ae 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -3281,10 +3281,10 @@ extract_range_from_binary_expr (value_range *vr,
the range of its operand *VR0 with type OP0_TYPE with resulting type TYPE.
The resulting range is stored in *VR. */
-static void
-extract_range_from_unary_expr_1 (value_range *vr,
- enum tree_code code, tree type,
- value_range *vr0_, tree op0_type)
+void
+extract_range_from_unary_expr_range (value_range *vr,
+ enum tree_code code, tree type,
+ value_range *vr0_, tree op0_type)
{
value_range vr0 = *vr0_, vrtem0 = VR_INITIALIZER, vrtem1 = VR_INITIALIZER;
@@ -3337,12 +3337,12 @@ extract_range_from_unary_expr_1 (value_range *vr,
if (vr0.type == VR_ANTI_RANGE
&& ranges_from_anti_range (&vr0, &vrtem0, &vrtem1))
{
- extract_range_from_unary_expr_1 (vr, code, type, &vrtem0, op0_type);
+ extract_range_from_unary_expr_range (vr, code, type, &vrtem0, op0_type);
if (vrtem1.type != VR_UNDEFINED)
{
value_range vrres = VR_INITIALIZER;
- extract_range_from_unary_expr_1 (&vrres, code, type,
- &vrtem1, op0_type);
+ extract_range_from_unary_expr_range (&vrres, code, type,
+ &vrtem1, op0_type);
vrp_meet (vr, &vrres);
}
return;
@@ -3597,7 +3597,7 @@ extract_range_from_unary_expr (value_range *vr, enum tree_code code,
else
set_value_range_to_varying (&vr0);
- extract_range_from_unary_expr_1 (vr, code, type, &vr0, TREE_TYPE (op0));
+ extract_range_from_unary_expr_range (vr, code, type, &vr0, TREE_TYPE (op0));
}
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index 7ffb7e7..ee92b58 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -51,4 +51,9 @@ struct GTY(()) value_range
extern void vrp_intersect_ranges (value_range *vr0, value_range *vr1);
extern void vrp_meet (value_range *vr0, const value_range *vr1);
extern void dump_value_range (FILE *, const value_range *);
+extern void extract_range_from_unary_expr_range (value_range *vr,
+ enum tree_code code,
+ tree type,
+ value_range *vr0_,
+ tree op0_type);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu
2016-09-24 7:11 ` kugan
@ 2016-09-26 7:45 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2016-09-26 7:45 UTC (permalink / raw)
To: kugan; +Cc: gcc-patches, Martin Jambor, Jan Hubicka
On Sat, Sep 24, 2016 at 5:36 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> There is also one more issue as reported by Pat Haugen. That is, in
> converting value_range of ssa (which we get from get_range_info) to
> argument type, my implementation is too simplistic and wrong at times. I can
> check TYPE_UNSIGNED here but that would be pessimistic.
>
> tree-vrp already has logic to handle this so the attached patch exports this
> and uses it.
>
> This also would be useful when we pass an argument of the function to anther
> function call with unary operation. I will send a separate patch for that
> later if this is OK.
>
> Bootstrapped and regression tested on x86_64-linux-gnu with no new
> regressions. Is this OK for trunk?
extract_range_from_unary_expr_range has range two times, please just
drop the _1 and have extract_range_from_unary_expr -- we have C++ now
which should deal with overloads just fine.
As followup you might want to rename the other _1 to overloads in
tree-vrp.c as well.
Ok with that change.
Thanks,
Richard.
> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2016-09-24 Kugan Vivekanandarajah <kuganv@linaro.org>
>
> PR ipa/77677
> * ipa-prop.c (ipa_compute_jump_functions_for_edge): Use
> extract_range_from_unary_expr to convert value_range.
> * tree-vrp.c (extract_range_from_unary_expr_1): Rename to.
> (extract_range_from_unary_expr): This.
> * tree-vrp.h (extract_range_from_unary_expr): Declare.
>
> gcc/testsuite/ChangeLog:
>
> 2016-09-24 Kugan Vivekanandarajah <kuganv@linaro.org>
>
> PR ipa/77677
> * gcc.dg/torture/pr77677-2.c: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-26 7:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-22 22:52 ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu kugan
2016-09-23 7:52 ` Richard Biener
2016-09-23 9:15 ` kugan
2016-09-23 9:37 ` Richard Biener
2016-09-24 7:11 ` kugan
2016-09-26 7:45 ` 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).