Re: [jakub@redhat.com: [3.3.2 PATCH] Backport invalid C++ tree sharing fix + testcase]
I'll prepare another upload on Sunday.
Daniel Jacobowitz writes:
> I'm pretty sure this is the patch we need.
>
> ----- Forwarded message from Jakub Jelinek <jakub@redhat.com> -----
>
> Date: Wed, 1 Oct 2003 20:35:19 +0200
> From: Jakub Jelinek <jakub@redhat.com>
> Subject: [3.3.2 PATCH] Backport invalid C++ tree sharing fix + testcase
> To: Mark Mitchell <mark@codesourcery.com>
> Cc: gcc-patches@gcc.gnu.org
> Reply-To: Jakub Jelinek <jakub@redhat.com>
>
> Hi!
>
> The following testcase is miscompiled on IA-32 (foo has an endless loop)
> in 3.3.x.
> It is a regression from 3.2.x and fixed again on the trunk.
> I tracked it down to:
> http://gcc.gnu.org/ml/gcc-patches/2003-07/msg01063.html
> Below is backport of the patch to gcc-3_3-branch.
> Ok to commit?
>
> 2003-07-09 Mark Mitchell <mark@codesourcery.com>
>
> * cp-tree.h (break_out_calls): Remove declaration.
> * tree.c (break_out_calls): Remove.
> * typeck.c (build_modify_expr): Avoid invalid sharing of trees.
>
> 2003-10-01 Jakub Jelinek <jakub@redhat.com>
>
> * g++.dg/opt/cond1.C: New test.
>
> --- gcc/testsuite/g++.dg/opt/cond1.C.jj 2003-09-15 15:40:47.000000000 +0200
> +++ gcc/testsuite/g++.dg/opt/cond1.C 2003-10-01 14:39:32.000000000 +0200
> @@ -0,0 +1,24 @@
> +// { dg-do run }
> +// { dg-options "-O2" }
> +
> +struct D { int x; };
> +struct W
> +{
> + W () {}
> + D & operator * () { return d; }
> + D d;
> +};
> +
> +int
> +foo (int y)
> +{
> + W m;
> + (*m).x = (y > 1 ? y : 0);
> + return (*m).x;
> +}
> +
> +int
> +main ()
> +{
> + return (foo (6) != 6);
> +}
> --- gcc/cp/cp-tree.h.jj 2003-10-01 12:08:54.000000000 +0200
> +++ gcc/cp/cp-tree.h 2003-10-01 22:23:33.000000000 +0200
> @@ -4317,7 +4317,6 @@ extern tree build_min PARAMS ((enum t
> extern tree build_min_nt PARAMS ((enum tree_code, ...));
> extern tree build_cplus_new PARAMS ((tree, tree));
> extern tree get_target_expr PARAMS ((tree));
> -extern tree break_out_calls PARAMS ((tree));
> extern tree build_cplus_method_type PARAMS ((tree, tree, tree));
> extern tree build_cplus_staticfn_type PARAMS ((tree, tree, tree));
> extern tree build_cplus_array_type PARAMS ((tree, tree));
> --- gcc/cp/typeck.c.jj 2003-09-16 17:25:55.000000000 +0200
> +++ gcc/cp/typeck.c 2003-10-01 22:23:06.000000000 +0200
> @@ -5502,44 +5502,8 @@ build_modify_expr (lhs, modifycode, rhs)
> if (newrhs == error_mark_node)
> return error_mark_node;
>
> - if (TREE_CODE (newrhs) == COND_EXPR)
> - {
> - tree lhs1;
> - tree cond = TREE_OPERAND (newrhs, 0);
> -
> - if (TREE_SIDE_EFFECTS (lhs))
> - cond = build_compound_expr (tree_cons
> - (NULL_TREE, lhs,
> - build_tree_list (NULL_TREE, cond)));
> -
> - /* Cannot have two identical lhs on this one tree (result) as preexpand
> - calls will rip them out and fill in RTL for them, but when the
> - rtl is generated, the calls will only be in the first side of the
> - condition, not on both, or before the conditional jump! (mrs) */
> - lhs1 = break_out_calls (lhs);
> -
> - if (lhs == lhs1)
> - /* If there's no change, the COND_EXPR behaves like any other rhs. */
> - result = build (modifycode == NOP_EXPR ? MODIFY_EXPR : INIT_EXPR,
> - lhstype, lhs, newrhs);
> - else
> - {
> - tree result_type = TREE_TYPE (newrhs);
> - /* We have to convert each arm to the proper type because the
> - types may have been munged by constant folding. */
> - result
> - = build (COND_EXPR, result_type, cond,
> - build_modify_expr (lhs, modifycode,
> - cp_convert (result_type,
> - TREE_OPERAND (newrhs, 1))),
> - build_modify_expr (lhs1, modifycode,
> - cp_convert (result_type,
> - TREE_OPERAND (newrhs, 2))));
> - }
> - }
> - else
> - result = build (modifycode == NOP_EXPR ? MODIFY_EXPR : INIT_EXPR,
> - lhstype, lhs, newrhs);
> + result = build (modifycode == NOP_EXPR ? MODIFY_EXPR : INIT_EXPR,
> + lhstype, lhs, newrhs);
>
> TREE_SIDE_EFFECTS (result) = 1;
>
> --- gcc/cp/tree.c.jj 2003-05-16 00:06:44.000000000 +0200
> +++ gcc/cp/tree.c 2003-10-01 22:24:46.000000000 +0200
> @@ -375,97 +375,6 @@ get_target_expr (init)
> {
> return build_target_expr_with_type (init, TREE_TYPE (init));
> }
> -
> -/* Recursively perform a preorder search EXP for CALL_EXPRs, making
> - copies where they are found. Returns a deep copy all nodes transitively
> - containing CALL_EXPRs. */
> -
> -tree
> -break_out_calls (exp)
> - tree exp;
> -{
> - register tree t1, t2 = NULL_TREE;
> - register enum tree_code code;
> - register int changed = 0;
> - register int i;
> -
> - if (exp == NULL_TREE)
> - return exp;
> -
> - code = TREE_CODE (exp);
> -
> - if (code == CALL_EXPR)
> - return copy_node (exp);
> -
> - /* Don't try and defeat a save_expr, as it should only be done once. */
> - if (code == SAVE_EXPR)
> - return exp;
> -
> - switch (TREE_CODE_CLASS (code))
> - {
> - default:
> - abort ();
> -
> - case 'c': /* a constant */
> - case 't': /* a type node */
> - case 'x': /* something random, like an identifier or an ERROR_MARK. */
> - return exp;
> -
> - case 'd': /* A decl node */
> -#if 0 /* This is bogus. jason 9/21/94 */
> -
> - t1 = break_out_calls (DECL_INITIAL (exp));
> - if (t1 != DECL_INITIAL (exp))
> - {
> - exp = copy_node (exp);
> - DECL_INITIAL (exp) = t1;
> - }
> -#endif
> - return exp;
> -
> - case 'b': /* A block node */
> - {
> - /* Don't know how to handle these correctly yet. Must do a
> - break_out_calls on all DECL_INITIAL values for local variables,
> - and also break_out_calls on all sub-blocks and sub-statements. */
> - abort ();
> - }
> - return exp;
> -
> - case 'e': /* an expression */
> - case 'r': /* a reference */
> - case 's': /* an expression with side effects */
> - for (i = TREE_CODE_LENGTH (code) - 1; i >= 0; i--)
> - {
> - t1 = break_out_calls (TREE_OPERAND (exp, i));
> - if (t1 != TREE_OPERAND (exp, i))
> - {
> - exp = copy_node (exp);
> - TREE_OPERAND (exp, i) = t1;
> - }
> - }
> - return exp;
> -
> - case '<': /* a comparison expression */
> - case '2': /* a binary arithmetic expression */
> - t2 = break_out_calls (TREE_OPERAND (exp, 1));
> - if (t2 != TREE_OPERAND (exp, 1))
> - changed = 1;
> - case '1': /* a unary arithmetic expression */
> - t1 = break_out_calls (TREE_OPERAND (exp, 0));
> - if (t1 != TREE_OPERAND (exp, 0))
> - changed = 1;
> - if (changed)
> - {
> - if (TREE_CODE_LENGTH (code) == 1)
> - return build1 (code, TREE_TYPE (exp), t1);
> - else
> - return build (code, TREE_TYPE (exp), t1, t2);
> - }
> - return exp;
> - }
> -
> -}
>
> /* Construct, lay out and return the type of methods belonging to class
> BASETYPE and whose arguments are described by ARGTYPES and whose values
>
> Jakub
>
>
> ----- End forwarded message -----
>
> --
> Daniel Jacobowitz
> MontaVista Software Debian GNU/Linux Developer
>
>
> --
> To UNSUBSCRIBE, email to debian-gcc-request@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Reply to: