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