Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu

[Richard Biener]

On Sat, Sep 24, 2016 at 5:36 AM, kugan
 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  
>
> 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  
>
> PR ipa/77677
> * gcc.dg/torture/pr77677-2.c: New test.

Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu

[kugan]

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  

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  

PR ipa/77677
* gcc.dg/torture/pr77677-2.c: New test.
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, , ))
- && (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 (>m_vr,
+  NOP_EXPR,
+  param_type,
+  , 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 (, , ))
 {
-  extract_range_from_unary_expr_1 (vr, code, type, , op0_type);
+  extract_range_from_unary_expr_range (vr, code, type, , op0_type);
   if (vrtem1.type != VR_UNDEFINED)
{
  value_range vrres = VR_INITIALIZER;
- extract_range_from_unary_expr_1 (, code, type,
-  , op0_type);
+ extract_range_from_unary_expr_range (, code, type,
+  , op0_type);
  vrp_meet (vr, );
}
   return;
@@ -3597,7 +3597,7 @@ extract_range_from_unary_expr (value_range *vr, enum 
tree_code code,
   else
 set_value_range_to_varying ();
 
-  extract_range_from_unary_expr_1 (vr, code, type, , TREE_TYPE (op0));
+  extract_range_from_unary_expr_range (vr, code, type, , TREE_TYPE (op0));
 }
 
 
diff --git 

Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu

[Richard Biener]

On Fri, Sep 23, 2016 at 10:58 AM, kugan
 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
>>  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  
>>>
>>> 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  
>>>
>>> PR ipa/77677
>>> * gcc.dg/torture/pr77677.c: New test.

Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu

[kugan]

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
 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  

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  

PR ipa/77677
* gcc.dg/torture/pr77677.c: New test.

Re: ICE at -O1 and above in both 32-bit and 64-bit modes on x86_64-linux-gnu

[Richard Biener]

On Fri, Sep 23, 2016 at 12:24 AM, kugan
 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  
>
> 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  
>
> PR ipa/77677
> * gcc.dg/torture/pr77677.c: New test.