On Mon, Oct 31, 2016 at 11:45:08AM -0400, Jason Merrill wrote:
> Is the tree-inline.c patch OK for trunk?
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -1241,6 +1241,28 @@ copy_tree_body_r (tree *tp, int *walk_subtrees, void
> *data)
> *walk_subtrees = 0;
> return NULL;
> }
> + else if (TREE_CODE (*tp) == COND_EXPR)
> + {
> + tree cond = TREE_OPERAND (*tp, 0);
> + walk_tree (&cond, copy_tree_body_r, data, NULL);
> + cond = fold (cond);
> + if (TREE_CODE (cond) == INTEGER_CST)
> + {
> + /* Only copy the taken branch; for a C++ base constructor clone
> + inherited from a virtual base, copying the other branch leads
> + to references to parameters that were optimized away. */
> + tree branch = (integer_nonzerop (cond)
> + ? TREE_OPERAND (*tp, 1)
> + : TREE_OPERAND (*tp, 2));
> + tree type = TREE_TYPE (*tp);
> + if (VOID_TYPE_P (type)
> + || type == TREE_TYPE (branch))
> + {
> + *tp = branch;
> + return copy_tree_body_r (tp, walk_subtrees, data);
> + }
> + }
> + }
This doesn't look right to me. I believe if we walk_tree copy_tree_body_r
any operand, we have to walk all of them and *walk_subtrees = 0;, otherwise
we'll effectively walk and copy possibly huge condition twice (which can
have very bad compile time / memory effects if the condition has many
COND_EXPRs in it).
So I think if you don't return copy_tree_body_r (tp, walk_subtrees, data);,
you should do something like:
copy_tree_r (tp, walk_subtrees, NULL);
TREE_OPERAND (*tp, 0) = cond;
walk_tree (&TREE_OPERAND (*tp, 1), copy_tree_body_r, data, NULL);
walk_tree (&TREE_OPERAND (*tp, 2), copy_tree_body_r, data, NULL);
*walk_subtrees = 0;
and then somehow arrange to continue after the
copy_tree_r (tp, walk_subtrees, NULL);
a few lines later, in particular the TREE_SET_BLOCK stuff, and
remap_type stuff (dunno if goto, or conditionalize the copy_tree_r there
on *walk_subtrees != 0, or what).
The case of a constant cond is likely ok, tp should already initially
point to an operand of a newly copied tree, just with the old COND_EXPR,
so if we replace it with a branch and recurse that should handle all the
copying/remapping etc.
Jakub
