This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][C++] Fix PR43075, really remove all zero-sized stores
- From: Richard Guenther <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Cc: jason at redhat dot com
- Date: Tue, 16 Feb 2010 13:29:17 +0100 (CET)
- Subject: Re: [PATCH][C++] Fix PR43075, really remove all zero-sized stores
- References: <alpine.LNX.2.00.1002151742050.23817@zhemvz.fhfr.qr>
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);