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  <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

Reply via email to