On 3/13/25 3:54 AM, Richard Biener wrote:
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.

No objection.

Though I notice that the documentation of DECL_NOT_GIMPLE_REG_P seems backwards?

Jason

Reply via email to