Re: [PATCH] Avoid IPA-CP versioning if type mismatch prevents it (PR tree-optimization/47967)
On Fri, Mar 4, 2011 at 9:48 PM, Jakub Jelinek wrote:
> Hi!
>
> If conversion between parameter type and passed argument is not useless,
> is not fold convertible and has different size, then even VCE is not
> appropriate, but for something so undefined we shouldn't be IMHO
> trying to optimize anything.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
> 2011-03-04 Jakub Jelinek
>
> PR tree-optimization/47967
> * ipa-cp.c (build_const_val): Return NULL instead of creating
> VIEW_CONVERT_EXPR for mismatching sizes.
> (ipcp_create_replace_map): Return NULL if build_const_val failed.
> (ipcp_insert_stage): If ipcp_create_replace_map returns NULL,
> give up on versioning.
>
> * gcc.c-torture/compile/pr47967.c: New test.
>
> --- gcc/ipa-cp.c.jj 2011-02-15 15:34:33.0 +0100
> +++ gcc/ipa-cp.c 2011-03-04 14:29:23.0 +0100
> @@ -587,8 +587,9 @@ ipcp_initialize_node_lattices (struct cg
> }
> }
>
> -/* build INTEGER_CST tree with type TREE_TYPE and value according to LAT.
> - Return the tree. */
> +/* Build a constant tree with type TREE_TYPE and value according to LAT.
> + Return the tree, or, if it is not possible to convert such value
> + to TREE_TYPE, NULL. */
> static tree
> build_const_val (struct ipcp_lattice *lat, tree tree_type)
> {
> @@ -601,8 +602,10 @@ build_const_val (struct ipcp_lattice *la
> {
> if (fold_convertible_p (tree_type, val))
> return fold_build1 (NOP_EXPR, tree_type, val);
> - else
> + else if (TYPE_SIZE (tree_type) == TYPE_SIZE (TREE_TYPE (val)))
> return fold_build1 (VIEW_CONVERT_EXPR, tree_type, val);
> + else
> + return NULL;
> }
> return val;
> }
> @@ -976,8 +979,20 @@ ipcp_create_replace_map (tree parm_tree,
> struct ipa_replace_map *replace_map;
> tree const_val;
>
> - replace_map = ggc_alloc_ipa_replace_map ();
> const_val = build_const_val (lat, TREE_TYPE (parm_tree));
> + if (const_val == NULL_TREE)
> + {
> + if (dump_file)
> + {
> + fprintf (dump_file, " const ");
> + print_generic_expr (dump_file, lat->constant, 0);
> + fprintf (dump_file, " can't be converted to param ");
> + print_generic_expr (dump_file, parm_tree, 0);
> + fprintf (dump_file, "\n");
> + }
> + return NULL;
> + }
> + replace_map = ggc_alloc_ipa_replace_map ();
> if (dump_file)
> {
> fprintf (dump_file, " replacing param ");
> @@ -1378,15 +1393,6 @@ ipcp_insert_stage (void)
> continue;
> }
>
> - new_size += growth;
> -
> - /* Look if original function becomes dead after cloning. */
> - for (cs = node->callers; cs != NULL; cs = cs->next_caller)
> - if (cs->caller == node || ipcp_need_redirect_p (cs))
> - break;
> - if (!cs && cgraph_will_be_removed_from_program_if_no_direct_calls
> (node))
> - bitmap_set_bit (dead_nodes, node->uid);
> -
> info = IPA_NODE_REF (node);
> count = ipa_get_param_count (info);
>
> @@ -1413,11 +1419,28 @@ ipcp_insert_stage (void)
> {
> replace_param =
> ipcp_create_replace_map (parm_tree, lat);
> + if (replace_param == NULL)
> + break;
> VEC_safe_push (ipa_replace_map_p, gc, replace_trees,
> replace_param);
> if (args_to_skip)
> bitmap_set_bit (args_to_skip, i);
> }
> }
> + if (i < count)
> + {
> + if (dump_file)
> + fprintf (dump_file, "Not versioning, some parameters couldn't be
> replaced");
> + continue;
> + }
> +
> + new_size += growth;
> +
> + /* Look if original function becomes dead after cloning. */
> + for (cs = node->callers; cs != NULL; cs = cs->next_caller)
> + if (cs->caller == node || ipcp_need_redirect_p (cs))
> + break;
> + if (!cs && cgraph_will_be_removed_from_program_if_no_direct_calls
> (node))
> + bitmap_set_bit (dead_nodes, node->uid);
>
> /* Compute how many callers node has. */
> node_callers = 0;
> --- gcc/testsuite/gcc.c-torture/compile/pr47967.c.jj 2011-03-04
> 14:31:29.0 +0100
> +++ gcc/testsuite/gcc.c-torture/compile/pr47967.c 2011-03-04
> 14:31:12.0 +0100
> @@ -0,0 +1,17 @@
> +/* PR tree-optimization/47967 */
> +
> +extern void abort (void);
> +static void bar ();
> +
> +void
> +foo ()
> +{
> + bar (1);
> +}
> +
> +static void
> +bar (double i)
> +{
> + if (i)
> + abort ();
> +}
>
> Jakub
>
[PATCH] Avoid IPA-CP versioning if type mismatch prevents it (PR tree-optimization/47967)
Hi!
If conversion between parameter type and passed argument is not useless,
is not fold convertible and has different size, then even VCE is not
appropriate, but for something so undefined we shouldn't be IMHO
trying to optimize anything.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2011-03-04 Jakub Jelinek
PR tree-optimization/47967
* ipa-cp.c (build_const_val): Return NULL instead of creating
VIEW_CONVERT_EXPR for mismatching sizes.
(ipcp_create_replace_map): Return NULL if build_const_val failed.
(ipcp_insert_stage): If ipcp_create_replace_map returns NULL,
give up on versioning.
* gcc.c-torture/compile/pr47967.c: New test.
--- gcc/ipa-cp.c.jj 2011-02-15 15:34:33.0 +0100
+++ gcc/ipa-cp.c2011-03-04 14:29:23.0 +0100
@@ -587,8 +587,9 @@ ipcp_initialize_node_lattices (struct cg
}
}
-/* build INTEGER_CST tree with type TREE_TYPE and value according to LAT.
- Return the tree. */
+/* Build a constant tree with type TREE_TYPE and value according to LAT.
+ Return the tree, or, if it is not possible to convert such value
+ to TREE_TYPE, NULL. */
static tree
build_const_val (struct ipcp_lattice *lat, tree tree_type)
{
@@ -601,8 +602,10 @@ build_const_val (struct ipcp_lattice *la
{
if (fold_convertible_p (tree_type, val))
return fold_build1 (NOP_EXPR, tree_type, val);
- else
+ else if (TYPE_SIZE (tree_type) == TYPE_SIZE (TREE_TYPE (val)))
return fold_build1 (VIEW_CONVERT_EXPR, tree_type, val);
+ else
+ return NULL;
}
return val;
}
@@ -976,8 +979,20 @@ ipcp_create_replace_map (tree parm_tree,
struct ipa_replace_map *replace_map;
tree const_val;
- replace_map = ggc_alloc_ipa_replace_map ();
const_val = build_const_val (lat, TREE_TYPE (parm_tree));
+ if (const_val == NULL_TREE)
+{
+ if (dump_file)
+ {
+ fprintf (dump_file, " const ");
+ print_generic_expr (dump_file, lat->constant, 0);
+ fprintf (dump_file, " can't be converted to param ");
+ print_generic_expr (dump_file, parm_tree, 0);
+ fprintf (dump_file, "\n");
+ }
+ return NULL;
+}
+ replace_map = ggc_alloc_ipa_replace_map ();
if (dump_file)
{
fprintf (dump_file, " replacing param ");
@@ -1378,15 +1393,6 @@ ipcp_insert_stage (void)
continue;
}
- new_size += growth;
-
- /* Look if original function becomes dead after cloning. */
- for (cs = node->callers; cs != NULL; cs = cs->next_caller)
- if (cs->caller == node || ipcp_need_redirect_p (cs))
- break;
- if (!cs && cgraph_will_be_removed_from_program_if_no_direct_calls (node))
- bitmap_set_bit (dead_nodes, node->uid);
-
info = IPA_NODE_REF (node);
count = ipa_get_param_count (info);
@@ -1413,11 +1419,28 @@ ipcp_insert_stage (void)
{
replace_param =
ipcp_create_replace_map (parm_tree, lat);
+ if (replace_param == NULL)
+ break;
VEC_safe_push (ipa_replace_map_p, gc, replace_trees,
replace_param);
if (args_to_skip)
bitmap_set_bit (args_to_skip, i);
}
}
+ if (i < count)
+ {
+ if (dump_file)
+ fprintf (dump_file, "Not versioning, some parameters couldn't be
replaced");
+ continue;
+ }
+
+ new_size += growth;
+
+ /* Look if original function becomes dead after cloning. */
+ for (cs = node->callers; cs != NULL; cs = cs->next_caller)
+ if (cs->caller == node || ipcp_need_redirect_p (cs))
+ break;
+ if (!cs && cgraph_will_be_removed_from_program_if_no_direct_calls (node))
+ bitmap_set_bit (dead_nodes, node->uid);
/* Compute how many callers node has. */
node_callers = 0;
--- gcc/testsuite/gcc.c-torture/compile/pr47967.c.jj2011-03-04
14:31:29.0 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr47967.c 2011-03-04
14:31:12.0 +0100
@@ -0,0 +1,17 @@
+/* PR tree-optimization/47967 */
+
+extern void abort (void);
+static void bar ();
+
+void
+foo ()
+{
+ bar (1);
+}
+
+static void
+bar (double i)
+{
+ if (i)
+abort ();
+}
Jakub
