[ipa-vrp] ice in set_value_range

Message ID fffd3860-e4c0-cf0d-4ac7-e4fc38e75fa6@linaro.org
State New
Headers show

Commit Message

kugan Oct. 28, 2016, 2:58 a.m.
Hi,

>>      {

>>        tree val = ipa_get_jf_constant (jfunc);

>>        if (TREE_CODE (val) == INTEGER_CST)

>>  	{

>> +	  value_range vr;

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

>> -	  jfunc->m_vr.max = val;

>> +

>> +	  vr.type = VR_RANGE;

>> +	  vr.min = val;

>> +	  vr.max = val;

>> +	  vr.equiv = NULL;

>> +	  extract_range_from_unary_expr (&jfunc->m_vr,

>> +					 NOP_EXPR,

>> +					 param_type,

>> +					 &vr, TREE_TYPE (val));

>

> Do I understand it correctly that extract_range_from_unary_expr deals

> with any potential type conversions better (compared to what you did

> before here)?


Yes, this can be wrong at times too as reported in 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78121. I have separated 
this part of the patch with a testcase.

Please note that I am using fold_convert in the attached patch.

Bootstrapped and regression tested on x86_64-linux-gnu with no new 
regressions. Is this OK for trunk?

Thanks,
Kugan


gcc/ChangeLog:

2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR ipa/78121
	* ipa-cp.c (propagate_vr_accross_jump_function): Pass param type.
	Also fold constant passed as argument while computing value range.
	(propagate_constants_accross_call): Pass param type.
	* ipa-prop.c: export ipa_get_callee_param_type.
	* ipa-prop.h: export ipa_get_callee_param_type.

gcc/testsuite/ChangeLog:

2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR ipa/78121
	* gcc.dg/ipa/pr78121.c: New test.

Comments

Martin Jambor Nov. 3, 2016, 4:24 p.m. | #1
Hi,

On Fri, Oct 28, 2016 at 01:58:13PM +1100, kugan wrote:
> > Do I understand it correctly that extract_range_from_unary_expr deals

> > with any potential type conversions better (compared to what you did

> > before here)?

> 

> Yes, this can be wrong at times too as reported in

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78121. I have separated this

> part of the patch with a testcase.

> 

> Please note that I am using fold_convert in the attached patch.

> 

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

> regressions. Is this OK for trunk?

> 


I have no objections, but we need to wait for Honza.

Thanks,

Martin

> Thanks,

> Kugan

> 

> 

> gcc/ChangeLog:

> 

> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

> 

> 	PR ipa/78121

> 	* ipa-cp.c (propagate_vr_accross_jump_function): Pass param type.

> 	Also fold constant passed as argument while computing value range.

> 	(propagate_constants_accross_call): Pass param type.

> 	* ipa-prop.c: export ipa_get_callee_param_type.

> 	* ipa-prop.h: export ipa_get_callee_param_type.

> 

> gcc/testsuite/ChangeLog:

> 

> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

> 

> 	PR ipa/78121

> 	* gcc.dg/ipa/pr78121.c: New test.
kugan Nov. 8, 2016, 10:11 a.m. | #2
Hi,

On 04/11/16 03:24, Martin Jambor wrote:
> Hi,

>

> On Fri, Oct 28, 2016 at 01:58:13PM +1100, kugan wrote:

>>> Do I understand it correctly that extract_range_from_unary_expr deals

>>> with any potential type conversions better (compared to what you did

>>> before here)?

>>

>> Yes, this can be wrong at times too as reported in

>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78121. I have separated this

>> part of the patch with a testcase.

>>

>> Please note that I am using fold_convert in the attached patch.

>>

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

>> regressions. Is this OK for trunk?

>>

>

> I have no objections, but we need to wait for Honza.

Thanks.

Honza, is this OK for you ?

Thanks,
Kugan

>

> Thanks,

>

> Martin

>

>> Thanks,

>> Kugan

>>

>>

>> gcc/ChangeLog:

>>

>> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

>>

>> 	PR ipa/78121

>> 	* ipa-cp.c (propagate_vr_accross_jump_function): Pass param type.

>> 	Also fold constant passed as argument while computing value range.

>> 	(propagate_constants_accross_call): Pass param type.

>> 	* ipa-prop.c: export ipa_get_callee_param_type.

>> 	* ipa-prop.h: export ipa_get_callee_param_type.

>>

>> gcc/testsuite/ChangeLog:

>>

>> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

>>

>> 	PR ipa/78121

>> 	* gcc.dg/ipa/pr78121.c: New test.

>
Jan Hubicka Nov. 8, 2016, 3:16 p.m. | #3
> Hi,

> 

> On 04/11/16 03:24, Martin Jambor wrote:

> >Hi,

> >

> >On Fri, Oct 28, 2016 at 01:58:13PM +1100, kugan wrote:

> >>>Do I understand it correctly that extract_range_from_unary_expr deals

> >>>with any potential type conversions better (compared to what you did

> >>>before here)?

> >>

> >>Yes, this can be wrong at times too as reported in

> >>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78121. I have separated this

> >>part of the patch with a testcase.

> >>

> >>Please note that I am using fold_convert in the attached patch.

> >>

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

> >>regressions. Is this OK for trunk?

> >>

> >

> >I have no objections, but we need to wait for Honza.

> Thanks.

> 

> Honza, is this OK for you ?

OK,
thanks!
Honza
> 

> Thanks,

> Kugan

> 

> >

> >Thanks,

> >

> >Martin

> >

> >>Thanks,

> >>Kugan

> >>

> >>

> >>gcc/ChangeLog:

> >>

> >>2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

> >>

> >>	PR ipa/78121

> >>	* ipa-cp.c (propagate_vr_accross_jump_function): Pass param type.

> >>	Also fold constant passed as argument while computing value range.

> >>	(propagate_constants_accross_call): Pass param type.

> >>	* ipa-prop.c: export ipa_get_callee_param_type.

> >>	* ipa-prop.h: export ipa_get_callee_param_type.

> >>

> >>gcc/testsuite/ChangeLog:

> >>

> >>2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

> >>

> >>	PR ipa/78121

> >>	* gcc.dg/ipa/pr78121.c: New test.

> >
Andrew Pinski Nov. 9, 2016, 6:02 a.m. | #4
On Tue, Nov 8, 2016 at 2:11 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote:
> Hi,

>

> On 04/11/16 03:24, Martin Jambor wrote:

>>

>> Hi,

>>

>> On Fri, Oct 28, 2016 at 01:58:13PM +1100, kugan wrote:

>>>>

>>>> Do I understand it correctly that extract_range_from_unary_expr deals

>>>> with any potential type conversions better (compared to what you did

>>>> before here)?

>>>

>>>

>>> Yes, this can be wrong at times too as reported in

>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78121. I have separated this

>>> part of the patch with a testcase.

>>>

>>> Please note that I am using fold_convert in the attached patch.

>>>

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

>>> regressions. Is this OK for trunk?

>>>

>>

>> I have no objections, but we need to wait for Honza.

>

> Thanks.

>

> Honza, is this OK for you ?



Either this patch or the patch for "Handle unary pass-through jump
functions for ipa-vrp" caused a bootstrap failure on
aarch64-linux-gnu.
Bootstrap comparison failure!
gcc/go/types.o differs
gcc/fortran/class.o differs
gcc/tree-ssa-live.o differs
gcc/data-streamer-out.o differs
gcc/ira-build.o differs
gcc/hsa-gen.o differs
gcc/hsa-brig.o differs
gcc/omp-low.o differs
gcc/lto-streamer-in.o differs
gcc/real.o differs
gcc/final.o differs
gcc/df-core.o differs

I bootstrap with the following options:

--with-cpu=thunderx+lse --enable-languages=c,c++,fortran,go
--disable-werror --with-sysroot=/ --enable-plugins
--enable-gnu-indirect-function

I have not tried removing the +lse part though

Thanks,
Andrew Pinski


>

> Thanks,

> Kugan

>

>

>>

>> Thanks,

>>

>> Martin

>>

>>> Thanks,

>>> Kugan

>>>

>>>

>>> gcc/ChangeLog:

>>>

>>> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

>>>

>>>         PR ipa/78121

>>>         * ipa-cp.c (propagate_vr_accross_jump_function): Pass param type.

>>>         Also fold constant passed as argument while computing value

>>> range.

>>>         (propagate_constants_accross_call): Pass param type.

>>>         * ipa-prop.c: export ipa_get_callee_param_type.

>>>         * ipa-prop.h: export ipa_get_callee_param_type.

>>>

>>> gcc/testsuite/ChangeLog:

>>>

>>> 2016-10-28  Kugan Vivekanandarajah  <kuganv@linaro.org>

>>>

>>>         PR ipa/78121

>>>         * gcc.dg/ipa/pr78121.c: New test.

>>

>>

>
kugan Nov. 9, 2016, 8:01 a.m. | #5
Hi Andrew,

On 09/11/16 17:02, Andrew Pinski wrote:
> Either this patch or the patch for "Handle unary pass-through jump

> functions for ipa-vrp" caused a bootstrap failure on

> aarch64-linux-gnu.

> Bootstrap comparison failure!

> gcc/go/types.o differs

> gcc/fortran/class.o differs

> gcc/tree-ssa-live.o differs

> gcc/data-streamer-out.o differs

> gcc/ira-build.o differs

> gcc/hsa-gen.o differs

> gcc/hsa-brig.o differs

> gcc/omp-low.o differs

> gcc/lto-streamer-in.o differs

> gcc/real.o differs

> gcc/final.o differs

> gcc/df-core.o differs

>

> I bootstrap with the following options:

>

> --with-cpu=thunderx+lse --enable-languages=c,c++,fortran,go

> --disable-werror --with-sysroot=/ --enable-plugins

> --enable-gnu-indirect-function

>

> I have not tried removing the +lse part though


Sorry about the breakage. I will try to reproduce this.

Thanks,
Kugan
Andrew Pinski Nov. 10, 2016, 6:14 a.m. | #6
On Wed, Nov 9, 2016 at 12:01 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Andrew,

>

> On 09/11/16 17:02, Andrew Pinski wrote:

>>

>> Either this patch or the patch for "Handle unary pass-through jump

>> functions for ipa-vrp" caused a bootstrap failure on

>> aarch64-linux-gnu.

>> Bootstrap comparison failure!

>> gcc/go/types.o differs

>> gcc/fortran/class.o differs

>> gcc/tree-ssa-live.o differs

>> gcc/data-streamer-out.o differs

>> gcc/ira-build.o differs

>> gcc/hsa-gen.o differs

>> gcc/hsa-brig.o differs

>> gcc/omp-low.o differs

>> gcc/lto-streamer-in.o differs

>> gcc/real.o differs

>> gcc/final.o differs

>> gcc/df-core.o differs

>>

>> I bootstrap with the following options:

>>

>> --with-cpu=thunderx+lse --enable-languages=c,c++,fortran,go

>> --disable-werror --with-sysroot=/ --enable-plugins

>> --enable-gnu-indirect-function

>>

>> I have not tried removing the +lse part though


I was able to reproduce it with just  --with-cpu=thunderx .  I am
trying without " --with-cpu=thunderx" now.  This is in my jenkins env
and I have not tried to reproduce it outside of it yet.

Thanks,
Andrew

>

>

> Sorry about the breakage. I will try to reproduce this.

>

> Thanks,

> Kugan
kugan Nov. 10, 2016, 6:18 a.m. | #7
Hi Andrew,

On 10/11/16 17:14, Andrew Pinski wrote:
> On Wed, Nov 9, 2016 at 12:01 AM, kugan

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

>> Hi Andrew,

>>

>> On 09/11/16 17:02, Andrew Pinski wrote:

>>>

>>> Either this patch or the patch for "Handle unary pass-through jump

>>> functions for ipa-vrp" caused a bootstrap failure on

>>> aarch64-linux-gnu.

>>> Bootstrap comparison failure!

>>> gcc/go/types.o differs

>>> gcc/fortran/class.o differs

>>> gcc/tree-ssa-live.o differs

>>> gcc/data-streamer-out.o differs

>>> gcc/ira-build.o differs

>>> gcc/hsa-gen.o differs

>>> gcc/hsa-brig.o differs

>>> gcc/omp-low.o differs

>>> gcc/lto-streamer-in.o differs

>>> gcc/real.o differs

>>> gcc/final.o differs

>>> gcc/df-core.o differs

>>>

>>> I bootstrap with the following options:

>>>

>>> --with-cpu=thunderx+lse --enable-languages=c,c++,fortran,go

>>> --disable-werror --with-sysroot=/ --enable-plugins

>>> --enable-gnu-indirect-function

>>>

>>> I have not tried removing the +lse part though

>

> I was able to reproduce it with just  --with-cpu=thunderx .  I am

> trying without " --with-cpu=thunderx" now.  This is in my jenkins env

> and I have not tried to reproduce it outside of it yet.


I can reproduce it. I am going to revert it as this is affecting your 
bootstrap. I will commit after fixing this (of-course after review)

Thanks,
Kugan

> Thanks,

> Andrew

>

>>

>>

>> Sorry about the breakage. I will try to reproduce this.

>>

>> Thanks,

>> Kugan
Andreas Schwab Nov. 10, 2016, 8:12 a.m. | #8
On Nov 09 2016, Andrew Pinski <pinskia@gmail.com> wrote:

> Either this patch or the patch for "Handle unary pass-through jump

> functions for ipa-vrp" caused a bootstrap failure on

> aarch64-linux-gnu.

> Bootstrap comparison failure!

> gcc/go/types.o differs

> gcc/fortran/class.o differs

> gcc/tree-ssa-live.o differs

> gcc/data-streamer-out.o differs

> gcc/ira-build.o differs

> gcc/hsa-gen.o differs

> gcc/hsa-brig.o differs

> gcc/omp-low.o differs

> gcc/lto-streamer-in.o differs

> gcc/real.o differs

> gcc/final.o differs

> gcc/df-core.o differs


Similar failure on ia64.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Patch hide | download patch | download mbox

From 64776c1b43c9fd68aee6c40624660d20e1c4e653 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Fri, 28 Oct 2016 09:44:23 +1100
Subject: [PATCH 1/2] fix convertion

---
 gcc/ipa-cp.c                       | 16 +++++++++++++---
 gcc/ipa-prop.c                     |  5 ++++-
 gcc/ipa-prop.h                     |  1 +
 gcc/testsuite/gcc.dg/ipa/pr78121.c | 16 ++++++++++++++++
 4 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr78121.c

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 1dc5cb6..9f28557 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1840,12 +1840,14 @@  propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j
 }
 
 /* Propagate value range across jump function JFUNC that is associated with
-   edge CS and update DEST_PLATS accordingly.  */
+   edge CS with param of callee of PARAM_TYPE and update DEST_PLATS
+   accordingly.  */
 
 static bool
 propagate_vr_accross_jump_function (cgraph_edge *cs,
 				    ipa_jump_func *jfunc,
-				    struct ipcp_param_lattices *dest_plats)
+				    struct ipcp_param_lattices *dest_plats,
+				    tree param_type)
 {
   struct ipcp_param_lattices *src_lats;
   ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range;
@@ -1853,6 +1855,11 @@  propagate_vr_accross_jump_function (cgraph_edge *cs,
   if (dest_lat->bottom_p ())
     return false;
 
+  if (!param_type
+      || (!INTEGRAL_TYPE_P (param_type)
+	  && !POINTER_TYPE_P (param_type)))
+    return dest_lat->set_to_bottom ();
+
   if (jfunc->type == IPA_JF_PASS_THROUGH)
     {
       struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
@@ -1871,6 +1878,7 @@  propagate_vr_accross_jump_function (cgraph_edge *cs,
 	{
 	  if (TREE_OVERFLOW_P (val))
 	    val = drop_tree_overflow (val);
+	  val = fold_convert (param_type, val);
 	  jfunc->vr_known = true;
 	  jfunc->m_vr.type = VR_RANGE;
 	  jfunc->m_vr.min = val;
@@ -2220,6 +2228,7 @@  propagate_constants_accross_call (struct cgraph_edge *cs)
     {
       struct ipa_jump_func *jump_func = ipa_get_ith_jump_func (args, i);
       struct ipcp_param_lattices *dest_plats;
+      tree param_type = ipa_get_callee_param_type (cs, i);
 
       dest_plats = ipa_get_parm_lattices (callee_info, i);
       if (availability == AVAIL_INTERPOSABLE)
@@ -2236,7 +2245,8 @@  propagate_constants_accross_call (struct cgraph_edge *cs)
 						       dest_plats);
 	  if (opt_for_fn (callee->decl, flag_ipa_vrp))
 	    ret |= propagate_vr_accross_jump_function (cs,
-						       jump_func, dest_plats);
+						       jump_func, dest_plats,
+						       param_type);
 	  else
 	    ret |= dest_plats->m_value_range.set_to_bottom ();
 	}
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 1629870..74fe199 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1595,7 +1595,10 @@  determine_locally_known_aggregate_parts (gcall *call, tree arg,
     }
 }
 
-static tree
+/* Return the Ith param type of callee associated with call graph
+   edge E.  */
+
+tree
 ipa_get_callee_param_type (struct cgraph_edge *e, int i)
 {
   int n;
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 4eeae88..0e75cf4 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -818,6 +818,7 @@  ipa_parm_adjustment *ipa_get_adjustment_candidate (tree **, bool *,
 						   ipa_parm_adjustment_vec,
 						   bool);
 void ipa_release_body_info (struct ipa_func_body_info *);
+tree ipa_get_callee_param_type (struct cgraph_edge *e, int i);
 
 /* From tree-sra.c:  */
 tree build_ref_for_offset (location_t, tree, HOST_WIDE_INT, bool, tree,
diff --git a/gcc/testsuite/gcc.dg/ipa/pr78121.c b/gcc/testsuite/gcc.dg/ipa/pr78121.c
new file mode 100644
index 0000000..4a0ae18
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr78121.c
@@ -0,0 +1,16 @@ 
+/* PR ipa/78121 */
+/* { dg-do compile } */
+/* { dg-options "-ansi -O2 -fdump-ipa-cp-details" } */
+
+void fn2 (unsigned char c);
+int a;
+static void fn1(c) unsigned char c;
+{
+    if (a)
+          fn2 (c);
+      fn1('#');
+}
+
+void fn3() { fn1 (267); }
+
+/* { dg-final { scan-ipa-dump-times "Setting value range of param 0 \\\[11, 35\\\]" 1 "cp" } } */
-- 
2.7.4