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?
2025-03-13 Jakub Jelinek <[email protected]>
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