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