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: [PATCH][C++] Fix PR43075, really remove all zero-sized stores


On Mon, 15 Feb 2010, Richard Guenther wrote:

> 
> This is an attempt (*sigh*) to fix PR43075 by making sure we remove
> zero-sized assignments that are not seen as zero-size from the IL
> during genericization.  We already do so for MODIFY_EXPR, but as
> PR43075 shows we may have INIT_EXPRs of this kind as well.
> 
> Now - simply adding || TREE_CODE (stmt) == INIT_EXPR break a shitload
> of code because there's funny things going on all over the place
> I have no idea of.

It's TARGET_EXPRs and AGGR_INIT_EXPRs that have special semantics
and do not like to be a statement of their own.  The issue is
that we have to preserve side-effects (and simply dropping the
RHS for INIT_EXPRs miscompiles stuff - as expected).

Is there a way to reduce a TARGET_EXPR or an AGGR_INIT_EXPR to
its side-effects?  I suppose TARGET_EXPR_INITIAL might be
that for TARGET_EXPR (unless it's an AGGR_INIT_EXPR itself).
For AGGR_INIT_EXPR it should be the initialization call,
but somehow it doesn't simply work to retain the AGGR_INIT_EXPR
as separate statement.

We probably shouldn't delay lowering TARGET_EXPR and AGGR_INIT_EXPR
details to gimplification but already perform that during
genericization.  I don't know if that would help this particular
case though.

> Thus, the following crude patch emerged from incrementally adding
> exceptions .... which now leads me to the question how
> cp_expr_size can say integer_zero_node to sth initialized from a
> AGGR_INIT_EXPR with obviously non-zero-sized elements.
> 
> Bah.  Maybe the case in PR43075 should have used a MODIFY_EXPR
> instead of an INIT_EXPR?
> 
> Jason, maybe you have more clue about the workings of cp_expr_size
> and INIT_EXPR - the gimplifier treats them the same as MODIFY_EXPR.
> 
> Thus, help!  (This one passes dg.exp at least, full bootstrap
> and regtest running)

It passed a full bootstrap and regtest on x86_64-unknown-linux-gnu.
But I still don't like it (tem and the check for NULL return
of cp_expr_size can be dropped and the patch still works).

Below is what I was playing with, but as I said it doesn't really
work - we still get odd trees for some g++.dg testcases.

Thanks,
Richard.

Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c	(revision 156802)
+++ gcc/cp/cp-gimplify.c	(working copy)
@@ -888,6 +888,34 @@ cp_genericize_r (tree *stmt_p, int *walk
 			TREE_OPERAND (stmt, 0),
 			TREE_OPERAND (stmt, 1));
     }
+  else if (TREE_CODE (stmt) == INIT_EXPR
+	   /* We have to preserve the result-decl initialization
+	      in a RETURN_EXPR.  */
+	   && TREE_CODE (TREE_OPERAND (stmt, 0)) != RESULT_DECL)
+    {
+      tree tem = cp_expr_size (TREE_OPERAND (stmt, 0));
+      if (tem
+	  && integer_zerop (tem))
+	{
+	  tree *rhsp = &TREE_OPERAND (stmt, 1);
+	  /* Look through but retain CLEANUP_POINT_EXPRs.  */
+	  if (TREE_CODE (*rhsp) == CLEANUP_POINT_EXPR)
+	    rhsp = &TREE_OPERAND (*rhsp, 0);
+	  /* Drop TARGET_EXPRs, retain their side-effect.  */
+	  if (TREE_CODE (*rhsp) == TARGET_EXPR)
+	    *rhsp = TARGET_EXPR_INITIAL (*rhsp);
+	  /* Move the zero-size LHS into AGGR_INIT_EXPRs.  */
+	  if (TREE_CODE (*rhsp) == AGGR_INIT_EXPR)
+	    {
+	      AGGR_INIT_EXPR_SLOT (*rhsp) = TREE_OPERAND (stmt, 0);
+	      *stmt_p = TREE_OPERAND (stmt, 1);
+	      TREE_TYPE (*stmt_p) = void_type_node;
+	    }
+	  else
+	    *stmt_p = build2 (COMPOUND_EXPR, TREE_TYPE (stmt),
+			      TREE_OPERAND (stmt, 0), TREE_OPERAND (stmt, 1));
+	}
+    }
 
   pointer_set_insert (p_set, *stmt_p);
 


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