This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC (gimple): PATCH to really avoid copying empty classes (c++/43787)


The first patch fixes the Ada problems that Eric pointed out; gimplify_modify_expr adds WITH_SIZE_EXPR to the RHS in some cases, so stripping that off shouldn't make us start over again.

And also I noticed that with the previous patch, if gimplify_modify_expr_rhs returns GS_OK but doesn't actually change *from, we would break out of the loop without actually producing gimple tuples.

The second patch is the cleanup I mentioned at the end of my previous patch mail, to make GS_OK always mean "keep trying." I fixed a few places that should have been setting GS_ALL_DONE and changed the default for the loop to make sure that all cases are setting 'ret' to something.

Tested x86_64-pc-linux-gnu with C, C++, Ada, Fortran, Java, ObjC, and LTO. Applying to trunk. I'll apply the first patch and my earlier patch to 4.5 next week if no other problems arise.
commit 2248226944b5bd5c665403951cb15e8b92cbfb80
Author: Jason Merrill <jason@redhat.com>
Date:   Wed May 5 17:57:12 2010 -0400

    	* gimplify.c (gimplify_modify_expr_rhs): Don't return GS_OK for
    	stripping WITH_SIZE_EXPR.
    	(gimplify_expr) [MODIFY_EXPR]: Trust GS_OK even if the rhs didn't
    	change.

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8d2bc58..51ec4b5 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4291,7 +4291,10 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
 	  if (TREE_CODE (TREE_OPERAND (*from_p, 0)) == CALL_EXPR)
 	    {
 	      *from_p = TREE_OPERAND (*from_p, 0);
-	      ret = GS_OK;
+	      /* We don't change ret in this case because the
+		 WITH_SIZE_EXPR might have been added in
+		 gimplify_modify_expr, so returning GS_OK would lead to an
+		 infinite loop.  */
 	      changed = true;
 	    }
 	  break;
@@ -6628,16 +6631,12 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 
 	case MODIFY_EXPR:
 	case INIT_EXPR:
-	  {
-	    tree from = TREE_OPERAND (*expr_p, 1);
-	    ret = gimplify_modify_expr (expr_p, pre_p, post_p,
-					fallback != fb_none);
-	    /* Don't let the end of loop logic change GS_OK into GS_ALL_DONE
-	       if the RHS has changed.  */
-	    if (ret == GS_OK && *expr_p == save_expr
-		&& TREE_OPERAND (*expr_p, 1) != from)
-	      continue;
-	  }
+	  ret = gimplify_modify_expr (expr_p, pre_p, post_p,
+				      fallback != fb_none);
+	  /* Don't let the end of loop logic change GS_OK to GS_ALL_DONE;
+	     gimplify_modify_expr_rhs might have changed the RHS.  */
+	  if (ret == GS_OK && *expr_p)
+	    continue;
 	  break;
 
 	case TRUTH_ANDIF_EXPR:
commit 68ef8e1d17c49580c2d4655aa2e976f10575c273
Author: Jason Merrill <jason@redhat.com>
Date:   Mon May 3 15:45:36 2010 -0400

    gcc:
    	* gimplify.c (gimplify_expr): Set GS_ALL_DONE when appropriate.
    	Don't change GS_OK to GS_ALL_DONE.  Make sure that all cases set
    	ret appropriately.
    	(gimplify_compound_lval): Return GS_ALL_DONE as appropriate.
    gcc/cp:
    	* semantics.c (simplify_aggr_init_expr): Use INIT_EXPR.

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f47a758..0847403 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3280,7 +3280,7 @@ simplify_aggr_init_expr (tree *tp)
 	 expand_call{,_inline}.  */
       cxx_mark_addressable (slot);
       CALL_EXPR_RETURN_SLOT_OPT (call_expr) = true;
-      call_expr = build2 (MODIFY_EXPR, TREE_TYPE (call_expr), slot, call_expr);
+      call_expr = build2 (INIT_EXPR, TREE_TYPE (call_expr), slot, call_expr);
     }
   else if (style == pcc)
     {
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 51ec4b5..f6266e1 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1914,9 +1914,10 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 {
   tree *p;
   VEC(tree,heap) *stack;
-  enum gimplify_status ret = GS_OK, tret;
+  enum gimplify_status ret = GS_ALL_DONE, tret;
   int i;
   location_t loc = EXPR_LOCATION (*expr_p);
+  tree expr = *expr_p;
 
   /* Create a stack of the subexpressions so later we can walk them in
      order from inner to outer.  */
@@ -2070,11 +2071,12 @@ gimplify_compound_lval (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
   if ((fallback & fb_rvalue) && TREE_CODE (*expr_p) == COMPONENT_REF)
     {
       canonicalize_component_ref (expr_p);
-      ret = MIN (ret, GS_OK);
     }
 
   VEC_free (tree, heap, stack);
 
+  gcc_assert (*expr_p == expr || ret != GS_ALL_DONE);
+
   return ret;
 }
 
@@ -6567,7 +6569,8 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
       else if (ret != GS_UNHANDLED)
 	break;
 
-      ret = GS_OK;
+      /* Make sure that all the cases set 'ret' appropriately.  */
+      ret = GS_UNHANDLED;
       switch (TREE_CODE (*expr_p))
 	{
 	  /* First deal with the special cases.  */
@@ -6601,6 +6604,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    {
 	      *expr_p = get_initialized_tmp_var (*expr_p, pre_p, post_p);
 	      mark_addressable (*expr_p);
+	      ret = GS_OK;
 	    }
 	  break;
 
@@ -6615,6 +6619,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    {
 	      *expr_p = get_initialized_tmp_var (*expr_p, pre_p, post_p);
 	      mark_addressable (*expr_p);
+	      ret = GS_OK;
 	    }
 	  break;
 
@@ -6633,10 +6638,6 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	case INIT_EXPR:
 	  ret = gimplify_modify_expr (expr_p, pre_p, post_p,
 				      fallback != fb_none);
-	  /* Don't let the end of loop logic change GS_OK to GS_ALL_DONE;
-	     gimplify_modify_expr_rhs might have changed the RHS.  */
-	  if (ret == GS_OK && *expr_p)
-	    continue;
 	  break;
 
 	case TRUTH_ANDIF_EXPR:
@@ -6680,6 +6681,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	      /* Just strip a conversion to void (or in void context) and
 		 try again.  */
 	      *expr_p = TREE_OPERAND (*expr_p, 0);
+	      ret = GS_OK;
 	      break;
 	    }
 
@@ -6700,7 +6702,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	case INDIRECT_REF:
 	  *expr_p = fold_indirect_ref_loc (input_location, *expr_p);
 	  if (*expr_p != save_expr)
-	    break;
+	    {
+	      ret = GS_OK;
+	      break;
+	    }
 	  /* else fall through.  */
 	case ALIGN_INDIRECT_REF:
 	case MISALIGNED_INDIRECT_REF:
@@ -6727,7 +6732,10 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  if (fallback & fb_lvalue)
 	    ret = GS_ALL_DONE;
 	  else
-	    *expr_p = DECL_INITIAL (*expr_p);
+	    {
+	      *expr_p = DECL_INITIAL (*expr_p);
+	      ret = GS_OK;
+	    }
 	  break;
 
 	case DECL_EXPR:
@@ -6762,6 +6770,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    }
 	  gimplify_seq_add_stmt (pre_p,
 			  gimple_build_goto (GOTO_DESTINATION (*expr_p)));
+	  ret = GS_ALL_DONE;
 	  break;
 
 	case PREDICT_EXPR:
@@ -6804,7 +6813,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 		  append_to_statement_list (ce->value, &temp);
 
 	      *expr_p = temp;
-	      ret = GS_OK;
+	      ret = temp ? GS_OK : GS_ALL_DONE;
 	    }
 	  /* C99 code may assign to an array in a constructed
 	     structure or union, and this has undefined behavior only
@@ -6814,6 +6823,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	    {
 	      *expr_p = get_initialized_tmp_var (*expr_p, pre_p, post_p);
 	      mark_addressable (*expr_p);
+	      ret = GS_OK;
 	    }
 	  else
 	    ret = GS_ALL_DONE;
@@ -6961,6 +6971,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 			   gimple_test_f, fallback);
 	    gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p, post_p,
 			   is_gimple_val, fb_rvalue);
+	    ret = GS_ALL_DONE;
 	  }
 	  break;
 
@@ -7048,6 +7059,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 		   TREE_TYPE (*expr_p))))
 	    {
 	      *expr_p = tmp;
+	      ret = GS_OK;
 	      break;
 	    }
 	  /* Convert (void *)&a + 4 into (void *)&a[1].  */
@@ -7063,6 +7075,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 					    0)))))
 	     {
                *expr_p = fold_convert (TREE_TYPE (*expr_p), tmp);
+	       ret = GS_OK;
 	       break;
 	     }
           /* FALLTHRU */
@@ -7132,9 +7145,7 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  break;
 	}
 
-      /* If we replaced *expr_p, gimplify again.  */
-      if (ret == GS_OK && (*expr_p == NULL || *expr_p == save_expr))
-	ret = GS_ALL_DONE;
+      gcc_assert (*expr_p || ret != GS_OK);
     }
   while (ret == GS_OK);
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]