Hi,
pass_split_functions is happy to split functions which have type
attributes but cannot update them if the new clone has in any way
different parameters than the original. This can lead to
miscompilations in cases like the testcase.
This patch solves it by 1) making the inliner set the
can_change_signature flag to false for them because their signature
cannot be changed (this step is also necessary to make IPA-CP operate
on them and handle them correctly), and 2) make the splitting pass
keep all parameters if the flag is set. The second step might involve
inventing some default definitions if the parameters did not really
have any.
I spoke about this with Honza and he claimed that the new function is
really an entirely different thing and that the parameters may
correspond only very loosely and thus the type attributes should be
cleared. I'm not sure I agree, but in any case I needed this to work
to allow me continue with promised IPA-CP polishing and so I decided
to do this because it was easier. (My own opinion is that the current
representation of parameter-describing function type attributes is
evil and will cause harm no matter hat we do.)
A very similar patch has passed bootstrap and testsuite on
x86_64-linux, the current one is undergoing both right now. OK for
trunk if it passes?
Thanks,
Martin
2011-07-28 Martin Jambor <mjam...@suse.cz>
PR middle-end/49886
* ipa-inline-analysis.c (compute_inline_parameters): Set
can_change_signature of noes with typde attributes.
* ipa-split.c (split_function): Do not skip any arguments if
can_change_signature is set.
* testsuite/gcc.c-torture/execute/pr49886.c: New testcase.
Index: src/gcc/ipa-inline-analysis.c
===================================================================
--- src.orig/gcc/ipa-inline-analysis.c
+++ src/gcc/ipa-inline-analysis.c
@@ -1658,18 +1658,24 @@ compute_inline_parameters (struct cgraph
/* Can this function be inlined at all? */
info->inlinable = tree_inlinable_function_p (node->decl);
- /* Inlinable functions always can change signature. */
- if (info->inlinable)
- node->local.can_change_signature = true;
+ /* Type attributes can use parameter indices to describe them. */
+ if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl)))
+ node->local.can_change_signature = false;
else
{
- /* Functions calling builtin_apply can not change signature. */
- for (e = node->callees; e; e = e->next_callee)
- if (DECL_BUILT_IN (e->callee->decl)
- && DECL_BUILT_IN_CLASS (e->callee->decl) == BUILT_IN_NORMAL
- && DECL_FUNCTION_CODE (e->callee->decl) == BUILT_IN_APPLY_ARGS)
- break;
- node->local.can_change_signature = !e;
+ /* Otherwise, inlinable functions always can change signature. */
+ if (info->inlinable)
+ node->local.can_change_signature = true;
+ else
+ {
+ /* Functions calling builtin_apply can not change signature. */
+ for (e = node->callees; e; e = e->next_callee)
+ if (DECL_BUILT_IN (e->callee->decl)
+ && DECL_BUILT_IN_CLASS (e->callee->decl) == BUILT_IN_NORMAL
+ && DECL_FUNCTION_CODE (e->callee->decl) == BUILT_IN_APPLY_ARGS)
+ break;
+ node->local.can_change_signature = !e;
+ }
}
estimate_function_body_sizes (node, early);
Index: src/gcc/ipa-split.c
===================================================================
--- src.orig/gcc/ipa-split.c
+++ src/gcc/ipa-split.c
@@ -945,10 +945,10 @@ static void
split_function (struct split_point *split_point)
{
VEC (tree, heap) *args_to_pass = NULL;
- bitmap args_to_skip = BITMAP_ALLOC (NULL);
+ bitmap args_to_skip;
tree parm;
int num = 0;
- struct cgraph_node *node;
+ struct cgraph_node *node, *cur_node = cgraph_get_node
(current_function_decl);
basic_block return_bb = find_return_bb ();
basic_block call_bb;
gimple_stmt_iterator gsi;
@@ -968,17 +968,30 @@ split_function (struct split_point *spli
dump_split_point (dump_file, split_point);
}
+ if (cur_node->local.can_change_signature)
+ args_to_skip = BITMAP_ALLOC (NULL);
+ else
+ args_to_skip = NULL;
+
/* Collect the parameters of new function and args_to_skip bitmap. */
for (parm = DECL_ARGUMENTS (current_function_decl);
parm; parm = DECL_CHAIN (parm), num++)
- if (!is_gimple_reg (parm)
- || !gimple_default_def (cfun, parm)
- || !bitmap_bit_p (split_point->ssa_names_to_pass,
- SSA_NAME_VERSION (gimple_default_def (cfun, parm))))
+ if (args_to_skip
+ && (!is_gimple_reg (parm)
+ || !gimple_default_def (cfun, parm)
+ || !bitmap_bit_p (split_point->ssa_names_to_pass,
+ SSA_NAME_VERSION (gimple_default_def (cfun,
+ parm)))))
bitmap_set_bit (args_to_skip, num);
else
{
arg = gimple_default_def (cfun, parm);
+ if (!arg)
+ {
+ arg = make_ssa_name (parm, gimple_build_nop ());
+ set_default_def (parm, arg);
+ }
+
if (TYPE_MAIN_VARIANT (DECL_ARG_TYPE (parm))
!= TYPE_MAIN_VARIANT (TREE_TYPE (arg)))
{
@@ -1081,9 +1094,7 @@ split_function (struct split_point *spli
/* Now create the actual clone. */
rebuild_cgraph_edges ();
- node = cgraph_function_versioning (cgraph_get_node (current_function_decl),
- NULL, NULL,
- args_to_skip,
+ node = cgraph_function_versioning (cur_node, NULL, NULL, args_to_skip,
split_point->split_bbs,
split_point->entry_bb, "part");
/* For usual cloning it is enough to clear builtin only when signature
@@ -1094,7 +1105,7 @@ split_function (struct split_point *spli
DECL_BUILT_IN_CLASS (node->decl) = NOT_BUILT_IN;
DECL_FUNCTION_CODE (node->decl) = (enum built_in_function) 0;
}
- cgraph_node_remove_callees (cgraph_get_node (current_function_decl));
+ cgraph_node_remove_callees (cur_node);
if (!split_part_return_p)
TREE_THIS_VOLATILE (node->decl) = 1;
if (dump_file)
Index: src/gcc/testsuite/gcc.c-torture/execute/pr49886.c
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gcc.c-torture/execute/pr49886.c
@@ -0,0 +1,100 @@
+struct PMC {
+ unsigned flags;
+};
+
+typedef struct Pcc_cell
+{
+ struct PMC *p;
+ long bla;
+ long type;
+} Pcc_cell;
+
+int gi;
+int cond;
+
+extern void abort ();
+extern void never_ever(int interp, struct PMC *pmc)
+ __attribute__((noinline,noclone));
+
+void never_ever (int interp, struct PMC *pmc)
+{
+ abort ();
+}
+
+static void mark_cell(int * interp, Pcc_cell *c)
+ __attribute__((__nonnull__(1)));
+
+static void
+mark_cell(int * interp, Pcc_cell *c)
+{
+ if (!cond)
+ return;
+
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<18)))
+ never_ever(gi + 1, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<17)))
+ never_ever(gi + 2, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<16)))
+ never_ever(gi + 3, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<15)))
+ never_ever(gi + 4, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<14)))
+ never_ever(gi + 5, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<13)))
+ never_ever(gi + 6, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<12)))
+ never_ever(gi + 7, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<11)))
+ never_ever(gi + 8, c->p);
+ if (c && c->type == 4 && c->p
+ && !(c->p->flags & (1<<10)))
+ never_ever(gi + 9, c->p);
+}
+
+static void
+foo(int * interp, Pcc_cell *c)
+{
+ mark_cell(interp, c);
+}
+
+static struct Pcc_cell *
+__attribute__((noinline,noclone))
+getnull(void)
+{
+ return (struct Pcc_cell *) 0;
+}
+
+
+int main()
+{
+ int i;
+
+ cond = 1;
+ for (i = 0; i < 100; i++)
+ foo (&gi, getnull ());
+ return 0;
+}
+
+
+void
+bar_1 (int * interp, Pcc_cell *c)
+{
+ c->bla += 1;
+ mark_cell(interp, c);
+}
+
+void
+bar_2 (int * interp, Pcc_cell *c)
+{
+ c->bla += 2;
+ mark_cell(interp, c);
+}
+
