[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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: