Re: [PATCH]middle-end Use subregs to expand COMPLEX_EXPR to set the lowpart.

[Richard Biener <rguenther@suse.de>]

On Tue, 5 Jul 2022, Richard Sandiford wrote:

> Tamar Christina <Tamar.Christina@arm.com> writes:
> >> > so that the multiple_p test is skipped if the structure is undefined.
> >> 
> >> Actually, we should probably skip the constant_multiple_p test as well.
> >> Keeping it would only be meaningful for little-endian.
> >> 
> >> simplify_gen_subreg should alread do the necessary checks to make sure
> >> that the subreg can be formed (via validate_subreg).
> >> 
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
> > and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > 	* expmed.cc (store_bit_field):
> >         * expmed.cc (store_bit_field_1): Add parameter that indicates if value is
> > 	still undefined and if so emit a subreg move instead.
> > 	(store_integral_bit_field): Likewise.
> > 	(store_bit_field): Likewise.
> > 	* expr.h (write_complex_part): Likewise.
> > 	* expmed.h (store_bit_field): Add new parameter.
> > 	* builtins.cc (expand_ifn_atomic_compare_exchange_into_call): Use new
> > 	parameter.
> > 	(expand_ifn_atomic_compare_exchange): Likewise.
> > 	* calls.cc (store_unaligned_arguments_into_pseudos): Likewise.
> > 	* emit-rtl.cc (validate_subreg): Likewise.
> > 	* expr.cc (emit_group_store): Likewise.
> > 	(copy_blkmode_from_reg): Likewise.
> > 	(copy_blkmode_to_reg): Likewise.
> > 	(clear_storage_hints): Likewise.
> > 	(write_complex_part):  Likewise.
> > 	(emit_move_complex_parts): Likewise.
> > 	(expand_assignment): Likewise.
> > 	(store_expr): Likewise.
> > 	(store_field): Likewise.
> > 	(expand_expr_real_2): Likewise.
> > 	* ifcvt.cc (noce_emit_move_insn): Likewise.
> > 	* internal-fn.cc (expand_arith_set_overflow): Likewise.
> > 	(expand_arith_overflow_result_store): Likewise.
> > 	(expand_addsub_overflow): Likewise.
> > 	(expand_neg_overflow): Likewise.
> > 	(expand_mul_overflow): Likewise.
> > 	(expand_arith_overflow): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	* g++.target/aarch64/complex-init.C: New test.
> >
> > --- inline copy of patch ---
> >
> > [?]
> > diff --git a/gcc/testsuite/g++.target/aarch64/complex-init.C b/gcc/testsuite/g++.target/aarch64/complex-init.C
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..d3fd3e88d04a87bacf1c4ee74ce25282c6ff81e8
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.target/aarch64/complex-init.C
> > @@ -0,0 +1,40 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
> > +
> > +/*
> > +** _Z1fii:
> > +** ...
> > +** 	bfi	x0, x1, 32, 32
> > +** 	ret
> > +** ...
> 
> Sorry for the test nit, but: it shouldn't be necessary to add ...
> after the ret.  Same for the other tests.
> 
> OK with that change, thanks.

This has

      HOST_WIDE_INT regnum;
...
      else if (((constant_multiple_p (bitnum, regsize * BITS_PER_UNIT, 
&regnum)
                 && multiple_p (bitsize, regsize * BITS_PER_UNIT))
                || undefined_p)
               && known_ge (GET_MODE_BITSIZE (GET_MODE (op0)), bitsize))
        {
          sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
                                     regnum * regsize);

where regnum is used uninitialized when undefined_p.  An uninit
improvement of mine will diagnose this now.

What's the intended behavior here?  Use regnum = 0?

Please fix.

Thanks,
Richard.

> Richard
> 
> > +*/
> > +_Complex int f(int a, int b) {
> > +    _Complex int t = a + b * 1i;
> > +    return t;
> > +}
> > +
> > +/*
> > +** _Z2f2ii:
> > +** ...
> > +** 	bfi	x0, x1, 32, 32
> > +** 	ret
> > +** ...
> > +*/
> > +_Complex int f2(int a, int b) {
> > +    _Complex int t = {a, b};
> > +    return t;
> > +}
> > +
> > +/* 
> > +** _Z12f_convolutedii:
> > +** ...
> > +** 	bfi	x0, x1, 32, 32
> > +** 	ret
> > +** ...
> > +*/
> > +_Complex int f_convoluted(int a, int b) {
> > +    _Complex int t = (_Complex int)a;
> > +    __imag__ t = b;
> > +    return t;
> > +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)