On Thu, 13 Mar 2025, Jakub Jelinek wrote:
> Hi!
>
> On Wed, Mar 12, 2025 at 02:01:14PM +0100, Richard Biener wrote:
> > On Wed, 12 Mar 2025, Jakub Jelinek wrote:
> >
> > > On Tue, Mar 11, 2025 at 12:13:13PM +0100, Richard Biener wrote:
> > > > On Tue, 11 Mar 2025, Jakub Jelinek wrote:
> > > >
> > > > > On Tue, Mar 11, 2025 at 10:18:18AM +0100, Richard Biener wrote:
> > > > > > I think the patch as-is is more robust, but still - ugh ... I wonder
> > > > > > whether we can instead avoid introducing the COMPLEX_EXPR at all
> > > > > > at -O0?
> > > > >
> > > > > Can we set DECL_NOT_GIMPLE_REG_P at -O0 during gimplification (where
> > > > > we've already handled some uses/setters of it), at least when
> > > > > gimplify_modify_expr_complex_part sees {REAL,IMAG}PART_EXPR on
> > > > > {VAR,PARM,RESULT}_DECL?
> > > >
> > > > Yes, that should work for LHS __real / __imag.
> > >
> > > Unfortunately it doesn't.
> > >
> > > Although successfully bootstrapped on x86_64-linux and i686-linux,
> > > it caused g++.dg/cpp1z/decomp2.C, g++.dg/torture/pr109262.C and
> > > g++.dg/torture/pr88149.C regressions.
> > >
> > > Minimal testcase is -O0:
> > > void
> > > foo (float x, float y)
> > > {
> > > __complex__ float z = x + y * 1.0fi;
> > > __real__ z = 1.0f;
> > > }
> > > which ICEs with
> > > pr88149.c: In function ‘foo’:
> > > pr88149.c:2:1: error: non-register as LHS of binary operation
> > > 2 | foo (float x, float y)
> > > | ^~~
> > > z = COMPLEX_EXPR <_2, y.0>;
> > > pr88149.c:2:1: internal compiler error: ‘verify_gimple’ failed
> > > When the initialization is being gimplified, z is still
> > > not DECL_NOT_GIMPLE_REG_P and so is_gimple_reg is true for it and
> > > so it gimplifies it as
> > > z = COMPLEX_EXPR <_2, y.0>;
> > > later, instead of building
> > > _3 = IMAGPART_EXPR <z>;
> > > z = COMPLEX_EXPR <1.0e+0, _3>;
> > > like before, the patch forces z to be not a gimple reg and uses
> > > REALPART_EXPR <z> = 1.0e+0;
> > > but it is too late, nothing fixes up the gimplification of the
> > > COMPLEX_EXPR
> > > anymore.
> >
> > Ah, yeah - setting DECL_NOT_GIMPLE_REG_P "after the fact" doesn't work.
> >
> > > So, I think we'd really need to do it the old way with adjusted naming
> > > of the flag, so assume for all non-addressable
> > > VAR_DECLs/PARM_DECLs/RESULT_DECLs with COMPLEX_TYPE if (!optimize) they
> > > are DECL_NOT_GIMPLE_REG_P (perhaps with the exception of
> > > get_internal_tmp_var), and at some point (what) if at all optimize that
> > > away if the partial accesses aren't done.
> >
> > We could of course do that in is_gimple_reg (), but I'm not sure if
> > all places that would need to check do so. Alternatively gimplify
> >
> > __real x = ..
> >
> > into
> >
> > tem[DECL_NOT_GIMPLE_REG_P] = x;
> > __real tem = ...;
> > x = tem;
>
> We can't do that, that again causes the undesirable copying of often
> uninitialized part(s).
>
> > when 'x' is a is_gimple_reg? Of course for -O0 this would be quite bad.
> > Likewise for your idea - where would we do this optimization when not
> > optimizing?
> >
> > So it would need to be the frontend(s) setting DECL_NOT_GIMPLE_REG_P
> > when producing lvalue __real/__imag accesses?
>
> The following patch sets it in the FEs during genericization.
> I think Fortran doesn't have a way to modify just real or just complex
> part separately.
>
> In short, this patch is for code like
> _ComplexT __t;
> __real__ __t = __z.real();
> __imag__ __t = __z.imag();
> _M_value *= __t;
> return *this;
> at -O0 which used to appear widely even in libstdc++ before GCC 9
> and happens in real-world code. At -O0 for debug info reasons (see
> PR119190) we don't want to aggressively DCE statements and when we
> since r0-100845 try to rewrite vars with COMPLEX_TYPE into SSA form
> aggressively, the above results in copying of uninitialized data
> when expanding COMPLEX_EXPRs added so that the vars can be in SSA form.
> The patch detects during genericization the partial initialization and
> doesn't rewrite such vars to SSA at -O0. This has to be done before
> gimplification starts, otherwise e.g. the attached testcase ICEs.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
LGTM, please leave frontend maintainers a chance to comment though.
Thanks,
Richard.
> 2025-03-13 Jakub Jelinek <ja...@redhat.com>
>
> PR target/119120
> * c-gimplify.cc (c_genericize_control_r): Set DECL_NOT_GIMPLE_REG_P
> on {REAL,IMAG}PART_EXPR is_gimple_reg operand at -O0 if it is lhs
> of a MODIFY_EXPR.
>
> * cp-gimplify.cc (cp_genericize_r): Set DECL_NOT_GIMPLE_REG_P
> on {REAL,IMAG}PART_EXPR is_gimple_reg operand at -O0 if it is lhs
> of a MODIFY_EXPR.
>
> * c-c++-common/pr119120.c: New test.
>
> --- gcc/c-family/c-gimplify.cc.jj 2025-02-13 10:21:20.103421347 +0100
> +++ gcc/c-family/c-gimplify.cc 2025-03-12 15:15:57.286920683 +0100
> @@ -727,6 +727,18 @@ c_genericize_control_stmt (tree *stmt_p,
> static tree
> c_genericize_control_r (tree *stmt_p, int *walk_subtrees, void *data)
> {
> + tree stmt = *stmt_p;
> + /* Mark stores to parts of complex automatic non-addressable
> + variables as DECL_NOT_GIMPLE_REG_P for -O0. This can't be
> + done during gimplification. See PR119120. */
> + if (TREE_CODE (stmt) == MODIFY_EXPR
> + && (TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
> + || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR)
> + && !optimize
> + && DECL_P (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0))
> + && is_gimple_reg (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0)))
> + DECL_NOT_GIMPLE_REG_P (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0)) = 1;
> +
> c_genericize_control_stmt (stmt_p, walk_subtrees, data,
> c_genericize_control_r, NULL);
> return NULL;
> --- gcc/cp/cp-gimplify.cc.jj 2025-03-07 16:34:02.266388660 +0100
> +++ gcc/cp/cp-gimplify.cc 2025-03-12 16:04:29.197874236 +0100
> @@ -2277,6 +2277,18 @@ cp_genericize_r (tree *stmt_p, int *walk
> TREE_TYPE (stmt), TREE_OPERAND (stmt, 0));
> break;
>
> + case MODIFY_EXPR:
> + /* Mark stores to parts of complex automatic non-addressable
> + variables as DECL_NOT_GIMPLE_REG_P for -O0. This can't be
> + done during gimplification. See PR119120. */
> + if ((TREE_CODE (TREE_OPERAND (stmt, 0)) == REALPART_EXPR
> + || TREE_CODE (TREE_OPERAND (stmt, 0)) == IMAGPART_EXPR)
> + && !optimize
> + && DECL_P (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0))
> + && is_gimple_reg (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0)))
> + DECL_NOT_GIMPLE_REG_P (TREE_OPERAND (TREE_OPERAND (stmt, 0), 0)) = 1;
> + break;
> +
> default:
> if (IS_TYPE_OR_DECL_P (stmt))
> *walk_subtrees = 0;
> --- gcc/testsuite/c-c++-common/pr119120.c.jj 2025-03-12 15:58:17.716945271
> +0100
> +++ gcc/testsuite/c-c++-common/pr119120.c 2025-03-13 00:21:28.393936936
> +0100
> @@ -0,0 +1,40 @@
> +/* PR target/119120 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump "REALPART_EXPR <r> = " "optimized" } } */
> +/* { dg-final { scan-tree-dump "IMAGPART_EXPR <r> = " "optimized" } } */
> +/* { dg-final { scan-tree-dump "REALPART_EXPR <s> = " "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "(REAL|IMAG)PART_EXPR <t> = " "optimized"
> } } */
> +/* { dg-final { scan-tree-dump-not "(REAL|IMAG)PART_EXPR <u> = " "optimized"
> } } */
> +
> +__complex__ double
> +foo (void)
> +{
> + __complex__ double r;
> + __imag__ r = 2.0;
> + __real__ r = 1.0;
> + return r + 1.0;
> +}
> +
> +__complex__ float
> +bar (float x, float y)
> +{
> + __complex__ float s = x + y * 1.0fi;
> + __real__ s = 1.0f;
> + return s + 1.0f;
> +}
> +
> +__complex__ float
> +baz (float x, float y)
> +{
> + __complex__ float t = x + y * 1.0fi;
> + return t + 1.0f;
> +}
> +
> +__complex__ float
> +qux (__complex__ float x)
> +{
> + __complex__ float u;
> + u = x;
> + return u + 1.0f;
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
