diff mbox

[PR78365] ICE in determine_value_range, at tree-ssa-loo p-niter.c:413

Message ID 3a8d994c-ad91-d940-eb95-8f8664ca6e36@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Dec. 12, 2016, 7:57 a.m. UTC 
Hi Richard,

>> I am fine with the new patch but you'll need an approval from Honza

>> or Richi.

>>

>> I find it a bit saddening that we cannot really rely on

>> gimple_call_fntype but at least I do not see any other way.

>

> The patch looks somewhat backward.  It populates the param type from

> the callee but the only thing we really know is the _originating_ type

> from the callers DECL_ARGUMENTS (if the JF is based on a parameter


I am not sure I understood this. I think we get param_type from callees 
DECL_ARGUMENTS.

> which is the only case where we do not know the type of the actual

> value statically -- we only know the type we expect).

>

> So the type would be present only on IPA_JF_PASS_THROUGH JFs

> (? does that also cover modified params?).

>

> At propagation time when applying the jump-function we have to make

> sure to first convert the actual parameter value to that expected type

> (or drop to BOTTOM if we can't do that conversion due to excess mismatches).


 From ipa vrp point of view, that is what is being done with the patch 
(In this version, I also changed the POINTER_TYPE_P to use param_type). 
At the point we create ipa_compute_jump_functions_for_edge, we are 
covering the VR for arguments to param_type.

In propagate_vr_accross_jump_function also, we are doing the conversion.

Thanks,
Kugan

Comments

Richard Biener Dec. 13, 2016, 2:33 p.m. UTC | #1 
On Mon, Dec 12, 2016 at 8:57 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,

>

>>> I am fine with the new patch but you'll need an approval from Honza

>>> or Richi.

>>>

>>> I find it a bit saddening that we cannot really rely on

>>> gimple_call_fntype but at least I do not see any other way.

>>

>>

>> The patch looks somewhat backward.  It populates the param type from

>> the callee but the only thing we really know is the _originating_ type

>> from the callers DECL_ARGUMENTS (if the JF is based on a parameter

>

>

> I am not sure I understood this. I think we get param_type from callees

> DECL_ARGUMENTS.

>

>> which is the only case where we do not know the type of the actual

>> value statically -- we only know the type we expect).

>>

>> So the type would be present only on IPA_JF_PASS_THROUGH JFs

>> (? does that also cover modified params?).

>>

>> At propagation time when applying the jump-function we have to make

>> sure to first convert the actual parameter value to that expected type

>> (or drop to BOTTOM if we can't do that conversion due to excess

>> mismatches).

>

>

> From ipa vrp point of view, that is what is being done with the patch (In

> this version, I also changed the POINTER_TYPE_P to use param_type). At the

> point we create ipa_compute_jump_functions_for_edge, we are covering the VR

> for arguments to param_type.

>

> In propagate_vr_accross_jump_function also, we are doing the conversion.


Hum, I think what your patch does is only half-way correct.  I'm
looking a bit more
into how the IPA propagation works and it looks like it's not the JF
where the type
needs to be stored but the lattice itself -- we need to know at propagation time
what type we expect for the arguments.  And if there's a mismatch with the
actual type flowing through the JF then we need to convert or punt.  And looking
closer that type should get known during propagation as at that time we should
have the actual parameter types as seen in the function body.

And ipa_get_callee_param_type should go away - just use

  jfunc->param_type = TREE_TYPE (arg);

(well, and no need to store that redundant information in the jump
function -- it's
there for all but pass-through JFs by means of the JF -- and pass-through JFs
get their type at propagation time from the source lattice values)

So we just need to get propagation correct (and not fiddle strangely with types
in JF building).

Richard.

> Thanks,

> Kugan
diff mbox

Patch

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 2ec671f..9853467 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1846,11 +1846,11 @@  propagate_bits_accross_jump_function (cgraph_edge *cs, int idx, ipa_jump_func *j
 static bool
 propagate_vr_accross_jump_function (cgraph_edge *cs,
 				    ipa_jump_func *jfunc,
-				    struct ipcp_param_lattices *dest_plats,
-				    tree param_type)
+				    struct ipcp_param_lattices *dest_plats)
 {
   struct ipcp_param_lattices *src_lats;
   ipcp_vr_lattice *dest_lat = &dest_plats->m_value_range;
+  tree param_type = jfunc->param_type;
 
   if (dest_lat->bottom_p ())
     return false;
@@ -2247,7 +2247,6 @@  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)
@@ -2264,8 +2263,7 @@  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,
-						       param_type);
+						       jump_func, dest_plats);
 	  else
 	    ret |= dest_plats->m_value_range.set_to_bottom ();
 	}
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 642111d..d9a817a 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -1659,26 +1659,13 @@  determine_locally_known_aggregate_parts (gcall *call, tree arg,
 /* Return the Ith param type of callee associated with call graph
    edge E.  */
 
-tree
+static tree
 ipa_get_callee_param_type (struct cgraph_edge *e, int i)
 {
   int n;
-  tree type = (e->callee
-	       ? TREE_TYPE (e->callee->decl)
-	       : gimple_call_fntype (e->call_stmt));
-  tree t = TYPE_ARG_TYPES (type);
-
-  for (n = 0; n < i; n++)
-    {
-      if (!t)
-        break;
-      t = TREE_CHAIN (t);
-    }
-  if (t)
-    return TREE_VALUE (t);
   if (!e->callee)
-    return NULL;
-  t = DECL_ARGUMENTS (e->callee->decl);
+   return NULL;
+  tree t = DECL_ARGUMENTS (e->callee->decl);
   for (n = 0; n < i; n++)
     {
       if (!t)
@@ -1732,6 +1719,7 @@  ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	    useful_context = true;
 	}
 
+      jfunc->param_type = param_type;
       if (POINTER_TYPE_P (TREE_TYPE (arg)))
 	{
 	  bool addr_nonzero = false;
@@ -1748,8 +1736,8 @@  ipa_compute_jump_functions_for_edge (struct ipa_func_body_info *fbi,
 	    {
 	      jfunc->vr_known = true;
 	      jfunc->m_vr.type = VR_ANTI_RANGE;
-	      jfunc->m_vr.min = build_int_cst (TREE_TYPE (arg), 0);
-	      jfunc->m_vr.max = build_int_cst (TREE_TYPE (arg), 0);
+	      jfunc->m_vr.min = build_int_cst (param_type, 0);
+	      jfunc->m_vr.max = build_int_cst (param_type, 0);
 	      jfunc->m_vr.equiv = NULL;
 	    }
 	  else
@@ -4717,6 +4705,7 @@  ipa_write_jump_function (struct output_block *ob,
   int i, count;
 
   streamer_write_uhwi (ob, jump_func->type);
+  stream_write_tree (ob, jump_func->param_type, true);
   switch (jump_func->type)
     {
     case IPA_JF_UNKNOWN:
@@ -4800,6 +4789,7 @@  ipa_read_jump_function (struct lto_input_block *ib,
   int i, count;
 
   jftype = (enum jump_func_type) streamer_read_uhwi (ib);
+  jump_func->param_type = stream_read_tree (ib, data_in);
   switch (jftype)
     {
     case IPA_JF_UNKNOWN:
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 0e75cf4..eeb0f6b 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -182,6 +182,9 @@  struct GTY (()) ipa_jump_func
   bool vr_known;
   value_range m_vr;
 
+  /* Type of callee param corresponding to this jump_func.  */
+  tree  param_type;
+
   enum jump_func_type type;
   /* Represents a value of a jump function.  pass_through is used only in jump
      function context.  constant represents the actual constant in constant jump
@@ -818,7 +821,6 @@  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/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c
index e69de29..5180a01 100644
--- a/gcc/testsuite/gcc.dg/torture/pr78365.c
+++ b/gcc/testsuite/gcc.dg/torture/pr78365.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+
+int a, b, c;
+char d;
+static void fn1 (void *, int);
+int fn2 (int);
+
+void fn1 (cc, yh) void *cc;
+char yh;
+{
+  char y;
+  a = fn2(c - b + 1);
+  for (; y <= yh; y++)
+    ;
+}
+
+void fn3()
+{
+    fn1((void *)fn3, 1);
+    fn1((void *)fn3, d - 1);
+}