This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR78365] ICE in determine_value_range, at tree-ssa-loo p-niter.c:413
- From: Martin Jambor <mjambor at suse dot cz>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: kugan <kugan dot vivekanandarajah at linaro dot org>, Jan Hubicka <hubicka at ucw dot cz>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 6 Jan 2017 19:00:34 +0100
- Subject: Re: [PR78365] ICE in determine_value_range, at tree-ssa-loo p-niter.c:413
- Authentication-results: sourceware.org; auth=none
- References: <68396c67-fdcc-33ed-5463-a07502959865@linaro.org> <20161123153317.wjyjxgp6ltg5cp6t@virgil.suse.cz> <CAFiYyc2SmuubewhQQ_Tt1_C1g3+LXWuprJSdFBRHUVqcVMCjLg@mail.gmail.com> <4f03c618-081b-e5b8-5ef6-6abdfddd9d9b@linaro.org> <20161207100853.ae3qcpo5ycblqxau@virgil.suse.cz> <b2c492eb-31ad-4071-8666-3a569479b424@linaro.org> <20161209105127.mb3xasvrcipt2kqg@virgil.suse.cz> <CAFiYyc2SZ5=H+MMhcTPeUbS32ppzK+qWgT=9urxgLfW_JPgxbg@mail.gmail.com> <20161214101523.mxvcgfqdwcbdy737@virgil.suse.cz> <CAFiYyc1G5LbjDmXpiaSfnPzCDfwSEMqtsRFA5O4qsE_jkKjqOA@mail.gmail.com>
Hi,
On Wed, Dec 14, 2016 at 01:12:11PM +0100, Richard Biener wrote:
> On Wed, Dec 14, 2016 at 11:15 AM, Martin Jambor <mjambor@suse.cz> wrote:
> > ...
> > +/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to
> > + DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if
> > + the result is a range or an anti-range. */
> > +
> > +static bool
> > +ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr,
> > + enum tree_code operation,
> > + tree dst_type, tree src_type)
> > +{
> > + memset (dst_vr, 0, sizeof (*dst_vr));
>
> The memset is not necessary.
Apparently it is. Without it, I ended up with corrupted
dst->vr_bitmup. I got ICEs when I removed the memset and tracked it
down to:
(gdb) p dst_vr->equiv->first->next
$14 = (bitmap_element *) 0x16
after extract_range_from_unary_expr returns.
>
> > + extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type);
> > + if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE)
> > + return true;
> > + else
> > + return false;
> > +}
> > +
> > /* Propagate value range across jump function JFUNC that is associated with
> > edge CS with param of callee of PARAM_TYPE and update DEST_PLATS
> > accordingly. */
> > @@ -1849,7 +1866,6 @@ propagate_vr_accross_jump_function (cgraph_edge *cs,
> > 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;
> >
> > if (dest_lat->bottom_p ())
> > @@ -1862,31 +1878,23 @@ propagate_vr_accross_jump_function (cgraph_edge *cs,
> >
> > if (jfunc->type == IPA_JF_PASS_THROUGH)
> > {
> > - struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> > - int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
> > - src_lats = ipa_get_parm_lattices (caller_info, src_idx);
> > + enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc);
> >
> > - if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
> > - return dest_lat->meet_with (src_lats->m_value_range);
> > - else if (param_type
> > - && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
> > - == tcc_unary))
> > + if (TREE_CODE_CLASS (operation) == tcc_unary)
> > {
> > - value_range vr;
> > - memset (&vr, 0, sizeof (vr));
> > + struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
> > + int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
> > tree operand_type = ipa_get_type (caller_info, src_idx);
> > - enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc);
> > + struct ipcp_param_lattices *src_lats
> > + = ipa_get_parm_lattices (caller_info, src_idx);
> >
> > if (src_lats->m_value_range.bottom_p ())
> > return dest_lat->set_to_bottom ();
> > -
> > - extract_range_from_unary_expr (&vr,
> > - operation,
> > - param_type,
> > - &src_lats->m_value_range.m_vr,
> > - operand_type);
> > - if (vr.type == VR_RANGE
> > - || vr.type == VR_ANTI_RANGE)
> > + value_range vr;
> > + if (ipa_vr_operation_and_type_effects (&vr,
> > + &src_lats->m_value_range.m_vr,
> > + operation, param_type,
> > + operand_type))
> > return dest_lat->meet_with (&vr);
> > }
> > }
> > @@ -1906,8 +1914,11 @@ propagate_vr_accross_jump_function (cgraph_edge *cs,
> > }
> > }
> >
> > - if (jfunc->vr_known)
> > - return dest_lat->meet_with (&jfunc->m_vr);
> > + value_range vr;
> > + if (jfunc->vr_known
> > + && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR,
> > + param_type, jfunc->passed_type))
>
> but instead of a new jfunc->passed_type you can use TREE_TYPE (jfunc->m_vr.min)
> for example.
Great, thanks a lot for this suggestion. I have used that and removed
the new field addition from the patch and used your suggestion
instead.
>
> I notice that ipa_jump_func is badly laid out:
>
> struct GTY (()) ipa_jump_func
> {
> /* Aggregate contants description. See struct ipa_agg_jump_function and its
> description. */
> struct ipa_agg_jump_function agg;
>
> /* Information about zero/non-zero bits. */
> struct ipa_bits bits;
>
> /* Information about value range. */
> bool vr_known;
> value_range m_vr;
>
> 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
> functions and member_cst holds constant c++ member functions. */
> union jump_func_value
> {
> struct ipa_constant_data GTY ((tag ("IPA_JF_CONST"))) constant;
> struct ipa_pass_through_data GTY ((tag ("IPA_JF_PASS_THROUGH")))
> pass_through;
> struct ipa_ancestor_jf_data GTY ((tag ("IPA_JF_ANCESTOR"))) ancestor;
> } GTY ((desc ("%1.type"))) value;
> };
>
> vr_known should be moved to pack with type.
I have swapped their position in this patch because it does not affect
anything else.
> and ipa_bits::known should be moved out of it as well.
...and will try to come up with a patch doing this soon.
> I also think we do not use the equiv member of m_vr and thus that is
> a waste.
>
> Not sure if memory use of this struct is an issue.
We create it for each and every actual argument in every call
statement we track with IPA-CP et al, so it is visible in mem-stats of
LTO WPA of big programs.
I have bootstrapped, lto-bootstrapped and tested the following on
x86_64-linux. OK for trunk?
Thanks,
Martin
2017-01-04 Martin Jambor <mjambor@suse.cz>
PR ipa/78365
PR ipa/78599
* ipa-prop.h (ipa_jump_func): Swap positions of vr_known and m_vr.
* ipa-cp.c (ipa_vr_operation_and_type_effects): New function.
(propagate_vr_accross_jump_function): Use the above function for all
value range computations for pass-through jump functions and type
converasion from explicit value range values.
(ipcp_propagate_stage): Do not attempt to deduce types of formal
parameters from TYPE_ARG_TYPES.
* ipa-prop.c (ipa_write_jump_function): Remove trailing whitespace.
(ipa_write_node_info): Stream type of the actual argument.
(ipa_read_node_info): Likewise. Also remove trailing whitespace.
testsuite/
* gcc.dg/torture/pr78365.c: New test.
---
gcc/ipa-cp.c | 71 +++++++++++++++++-----------------
gcc/ipa-prop.c | 14 +++++--
gcc/ipa-prop.h | 5 ++-
gcc/testsuite/gcc.dg/torture/pr78365.c | 21 ++++++++++
4 files changed, 69 insertions(+), 42 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr78365.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 82bf35084b6..9cc903769e8 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1837,6 +1837,23 @@ propagate_bits_across_jump_function (cgraph_edge *cs, int idx,
return dest_lattice->set_to_bottom ();
}
+/* Emulate effects of unary OPERATION and/or conversion from SRC_TYPE to
+ DST_TYPE on value range in SRC_VR and store it to DST_VR. Return true if
+ the result is a range or an anti-range. */
+
+static bool
+ipa_vr_operation_and_type_effects (value_range *dst_vr, value_range *src_vr,
+ enum tree_code operation,
+ tree dst_type, tree src_type)
+{
+ memset (dst_vr, 0, sizeof (*dst_vr));
+ extract_range_from_unary_expr (dst_vr, operation, dst_type, src_vr, src_type);
+ if (dst_vr->type == VR_RANGE || dst_vr->type == VR_ANTI_RANGE)
+ return true;
+ else
+ return false;
+}
+
/* Propagate value range across jump function JFUNC that is associated with
edge CS with param of callee of PARAM_TYPE and update DEST_PLATS
accordingly. */
@@ -1846,7 +1863,6 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
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;
if (dest_lat->bottom_p ())
@@ -1859,31 +1875,23 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
if (jfunc->type == IPA_JF_PASS_THROUGH)
{
- struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
- int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
- src_lats = ipa_get_parm_lattices (caller_info, src_idx);
+ enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc);
- if (ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
- return dest_lat->meet_with (src_lats->m_value_range);
- else if (param_type
- && (TREE_CODE_CLASS (ipa_get_jf_pass_through_operation (jfunc))
- == tcc_unary))
+ if (TREE_CODE_CLASS (operation) == tcc_unary)
{
- value_range vr;
- memset (&vr, 0, sizeof (vr));
+ struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
+ int src_idx = ipa_get_jf_pass_through_formal_id (jfunc);
tree operand_type = ipa_get_type (caller_info, src_idx);
- enum tree_code operation = ipa_get_jf_pass_through_operation (jfunc);
+ struct ipcp_param_lattices *src_lats
+ = ipa_get_parm_lattices (caller_info, src_idx);
if (src_lats->m_value_range.bottom_p ())
return dest_lat->set_to_bottom ();
-
- extract_range_from_unary_expr (&vr,
- operation,
- param_type,
- &src_lats->m_value_range.m_vr,
- operand_type);
- if (vr.type == VR_RANGE
- || vr.type == VR_ANTI_RANGE)
+ value_range vr;
+ if (ipa_vr_operation_and_type_effects (&vr,
+ &src_lats->m_value_range.m_vr,
+ operation, param_type,
+ operand_type))
return dest_lat->meet_with (&vr);
}
}
@@ -1903,8 +1911,12 @@ propagate_vr_across_jump_function (cgraph_edge *cs, ipa_jump_func *jfunc,
}
}
- if (jfunc->vr_known)
- return dest_lat->meet_with (&jfunc->m_vr);
+ value_range vr;
+ if (jfunc->vr_known
+ && ipa_vr_operation_and_type_effects (&vr, &jfunc->m_vr, NOP_EXPR,
+ param_type,
+ TREE_TYPE (jfunc->m_vr.min)))
+ return dest_lat->meet_with (&vr);
else
return dest_lat->set_to_bottom ();
}
@@ -2244,7 +2256,7 @@ propagate_constants_across_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);
+ tree param_type = ipa_get_type (callee_info, i);
dest_plats = ipa_get_parm_lattices (callee_info, i);
if (availability == AVAIL_INTERPOSABLE)
@@ -3230,19 +3242,6 @@ ipcp_propagate_stage (struct ipa_topo_info *topo)
{
struct ipa_node_params *info = IPA_NODE_REF (node);
- /* In LTO we do not have PARM_DECLs but we would still like to be able to
- look at types of parameters. */
- if (in_lto_p)
- {
- tree t = TYPE_ARG_TYPES (TREE_TYPE (node->decl));
- for (int k = 0; k < ipa_get_param_count (info) && t; k++)
- {
- gcc_assert (t != void_list_node);
- info->descriptors[k].decl_or_type = TREE_VALUE (t);
- t = t ? TREE_CHAIN (t) : NULL;
- }
- }
-
determine_versionability (node, info);
if (node->has_gimple_body_p ())
{
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 1afa8fc7a05..1f68f736e46 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -4775,7 +4775,7 @@ ipa_write_jump_function (struct output_block *ob,
{
streamer_write_widest_int (ob, jump_func->bits.value);
streamer_write_widest_int (ob, jump_func->bits.mask);
- }
+ }
bp_pack_value (&bp, jump_func->vr_known, 1);
streamer_write_bitpack (&bp);
if (jump_func->vr_known)
@@ -4973,7 +4973,10 @@ ipa_write_node_info (struct output_block *ob, struct cgraph_node *node)
bp_pack_value (&bp, ipa_is_param_used (info, j), 1);
streamer_write_bitpack (&bp);
for (j = 0; j < ipa_get_param_count (info); j++)
- streamer_write_hwi (ob, ipa_get_controlled_uses (info, j));
+ {
+ streamer_write_hwi (ob, ipa_get_controlled_uses (info, j));
+ stream_write_tree (ob, ipa_get_type (info, j), true);
+ }
for (e = node->callees; e; e = e->next_callee)
{
struct ipa_edge_args *args = IPA_EDGE_REF (e);
@@ -5020,7 +5023,7 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node,
for (k = 0; k < ipa_get_param_count (info); k++)
info->descriptors[k].move_cost = streamer_read_uhwi (ib);
-
+
bp = streamer_read_bitpack (ib);
if (ipa_get_param_count (info) != 0)
info->analysis_done = true;
@@ -5028,7 +5031,10 @@ ipa_read_node_info (struct lto_input_block *ib, struct cgraph_node *node,
for (k = 0; k < ipa_get_param_count (info); k++)
ipa_set_param_used (info, k, bp_unpack_value (&bp, 1));
for (k = 0; k < ipa_get_param_count (info); k++)
- ipa_set_controlled_uses (info, k, streamer_read_hwi (ib));
+ {
+ ipa_set_controlled_uses (info, k, streamer_read_hwi (ib));
+ info->descriptors[k].decl_or_type = stream_read_tree (ib, data_in);
+ }
for (e = node->callees; e; e = e->next_callee)
{
struct ipa_edge_args *args = IPA_EDGE_REF (e);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 341d9db6c63..c9a69ab1f53 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -178,9 +178,10 @@ struct GTY (()) ipa_jump_func
/* Information about zero/non-zero bits. */
struct ipa_bits bits;
- /* Information about value range. */
- bool vr_known;
+ /* Information about value range, containing valid data only when vr_known is
+ true. */
value_range m_vr;
+ bool vr_known;
enum jump_func_type type;
/* Represents a value of a jump function. pass_through is used only in jump
diff --git a/gcc/testsuite/gcc.dg/torture/pr78365.c b/gcc/testsuite/gcc.dg/torture/pr78365.c
new file mode 100644
index 00000000000..5180a0171ae
--- /dev/null
+++ 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);
+}
--
2.11.0