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;
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?
Richard.
> Thoughts on this?
>
> 2025-03-11 Jakub Jelinek <ja...@redhat.com>
>
> PR target/119120
> * gimplify.cc (gimplify_modify_expr): Don't call
> gimplify_modify_expr_complex_part for -O0, instead set
> DECL_NOT_GIMPLE_REG_P on the {REAL,IMAG}PART_EXPR operand.
>
> --- gcc/gimplify.cc.jj 2025-03-10 09:31:20.579772627 +0100
> +++ gcc/gimplify.cc 2025-03-11 15:03:27.633636148 +0100
> @@ -7237,7 +7237,15 @@ gimplify_modify_expr (tree *expr_p, gimp
> if ((TREE_CODE (*to_p) == REALPART_EXPR
> || TREE_CODE (*to_p) == IMAGPART_EXPR)
> && is_gimple_reg (TREE_OPERAND (*to_p, 0)))
> - return gimplify_modify_expr_complex_part (expr_p, pre_p, want_value);
> + {
> + if (optimize)
> + return gimplify_modify_expr_complex_part (expr_p, pre_p, want_value);
> + /* When not optimizing, instead set DECL_NOT_GIMPLE_REG_P on it and
> + keep using {REAL,IMAG}PART_EXPR on the partial initializations
> + in order to avoid copying around uninitialized parts. See
> + PR119120. */
> + DECL_NOT_GIMPLE_REG_P (TREE_OPERAND (*to_p, 0)) = 1;
> + }
>
> /* Try to alleviate the effects of the gimplification creating artificial
> temporaries (see for example is_gimple_reg_rhs) on the debug info, but
>
>
> 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)
