I'm pretty sure this is the patch we need. ----- Forwarded message from Jakub Jelinek <[EMAIL PROTECTED]> -----
Date: Wed, 1 Oct 2003 20:35:19 +0200 From: Jakub Jelinek <[EMAIL PROTECTED]> Subject: [3.3.2 PATCH] Backport invalid C++ tree sharing fix + testcase To: Mark Mitchell <[EMAIL PROTECTED]> Cc: [EMAIL PROTECTED] Reply-To: Jakub Jelinek <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> * 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 <[EMAIL PROTECTED]> * 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
