On Tue, 23 Jul 2024, Jakub Jelinek wrote:
> Hi!
>
> The folding into REALPART_EXPR is correct, used only when the mem_offset
> is zero, but for IMAGPART_EXPR it didn't check the exact offset value (just
> that it is not 0).
> The following patch fixes that by using IMAGPART_EXPR only if the offset
> is right and using BITFIELD_REF or whatever else otherwise.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk, 14.2
> and other release branches?
I think this is not enough - what kind of GIMPLE does this result in?
You should amend the checking in non_rewritable_mem_ref_base as well,
it should fail when the corresponding rewrite doesn't work.
I suspect it falls through to the BIT_FIELD_REF code?
That said, can you match up the offset check with that of
non_rewritable_mem_ref_base then, particularly
if ((VECTOR_TYPE_P (TREE_TYPE (decl))
|| TREE_CODE (TREE_TYPE (decl)) == COMPLEX_TYPE)
&& useless_type_conversion_p (TREE_TYPE (base),
TREE_TYPE (TREE_TYPE (decl)))
&& known_ge (mem_ref_offset (base), 0)
&& known_gt (wi::to_poly_offset (TYPE_SIZE_UNIT (TREE_TYPE
(decl))),
mem_ref_offset (base))
&& multiple_p (mem_ref_offset (base),
wi::to_poly_offset (TYPE_SIZE_UNIT (TREE_TYPE
(base)))))
I suppose this check should be adjusted to use the three arg multiple_p
and check the factor against 0/1 for COMPLEX_TYPE.
It would probably better maintainance-wise to merge the checking
and transform routines and make it have two modes, check only or
check-and-transform. That makes it easier to keep them in sync.
Richard.
> 2024-07-23 Jakub Jelinek <ja...@redhat.com>
> Andrew Pinski <quic_apin...@quicinc.com>
>
> PR tree-optimization/116034
> * tree-sra.cc (maybe_rewrite_mem_ref_base): Only use IMAGPART_EXPR
> if MEM_REF offset is equal to element type size.
>
> * gcc.dg/pr116034.c: New test.
>
> --- gcc/tree-ssa.cc.jj 2024-03-11 11:00:46.768915988 +0100
> +++ gcc/tree-ssa.cc 2024-07-22 21:27:02.115530861 +0200
> @@ -1506,7 +1506,10 @@ maybe_rewrite_mem_ref_base (tree *tp, bi
> }
> else if (TREE_CODE (TREE_TYPE (sym)) == COMPLEX_TYPE
> && useless_type_conversion_p (TREE_TYPE (*tp),
> - TREE_TYPE (TREE_TYPE (sym))))
> + TREE_TYPE (TREE_TYPE (sym)))
> + && (integer_zerop (TREE_OPERAND (*tp, 1))
> + || tree_int_cst_equal (TREE_OPERAND (*tp, 1),
> + TYPE_SIZE_UNIT (TREE_TYPE (*tp)))))
> {
> *tp = build1 (integer_zerop (TREE_OPERAND (*tp, 1))
> ? REALPART_EXPR : IMAGPART_EXPR,
> --- gcc/testsuite/gcc.dg/pr116034.c.jj 2024-07-22 21:39:50.050994243
> +0200
> +++ gcc/testsuite/gcc.dg/pr116034.c 2024-07-22 21:39:32.432213042 +0200
> @@ -0,0 +1,21 @@
> +/* PR tree-optimization/116034 */
> +/* { dg-do run } */
> +/* { dg-options "-O1 -fno-strict-aliasing" } */
> +
> +int g;
> +
> +static inline int
> +foo (_Complex unsigned short c)
> +{
> + __builtin_memmove (&g, 1 + (char *) &c, 2);
> + return g;
> +}
> +
> +int
> +main ()
> +{
> + if (__SIZEOF_SHORT__ == 2
> + && __CHAR_BIT__ == 8
> + && foo (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ ? 0x0100 : 1) != 1)
> + __builtin_abort ();
> +}
>
> 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)
