On Tue, Apr 20, 2021 at 5:16 PM Andreas Krebbel via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> With the current handling of decl alignments it is impossible to
> reduce the alignment requirement as part of a variable declaration.
>
> This change has been proposed by Richard in the PR. It fixes the
> align-1.c testcase on IBM Z.
>
> Bootstrapped on x86_64 and s390x. No regressions.
>
> Ok for mainline?
I don't think this is a good fix since it leaves set_mem_attributes_minus_bitpos
in inconsistent handling across the different cases. The underlying
problem is the different expectation of mem:Mode with respect to
alignment on strict and not strict-alignment targets. On strict alignment
targets a mem:Mode guarantees alignment of at least the mode alignment
of Mode (well I don't think it does, but gossip says it has to) and thus for
smaller alignment such targets would need to use mem:BLK.
For the gcc.target/s390/vector/align-1.c we're likely ending up with
V4SImode mems regardless of the alignment of the access.
That would mean that a "conservative" (in accordance with the above
gossip) patch might be
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 07e908624a0..138ce75df94 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -1989,7 +1989,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree
t, int objectp,
/* Respect mode alignment for STRICT_ALIGNMENT targets if T is a type;
if T is an object, always compute the object alignment below. */
- if (TYPE_P (t))
+ if (TYPE_P (t) && (!objectp || STRICT_ALIGNMENT))
attrs.align = defattrs->align;
else
attrs.align = BITS_PER_UNIT;
in particular the !objectp check is required for the following
/* We can set the alignment from the type if we are making an object or if
this is an INDIRECT_REF. */
if (objectp || TREE_CODE (t) == INDIRECT_REF)
attrs.align = MAX (attrs.align, TYPE_ALIGN (type));
to allow an alignment less than the mode alignment. The || STRICT_ALIGNMENT
is to make sure alignment will not end up less than the modes alignment on
strict alignment targets.
Does that help as well? I think this one would be good to go.
Thanks,
Richard.
> gcc/ChangeLog:
>
> PR middle-end/88085
> * emit-rtl.c (set_mem_attributes_minus_bitpos): Use the user
> alignment if there are no pre-existing mem attrs.
> ---
> gcc/emit-rtl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index 07e908624a0..fc12fa927da 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -2124,7 +2124,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> objectp,
> unsigned int diff_align = known_alignment (obj_bitpos - bitpos);
> if (diff_align != 0)
> obj_align = MIN (obj_align, diff_align);
> - attrs.align = MAX (attrs.align, obj_align);
> + attrs.align = refattrs ? MAX (refattrs->align, obj_align) : obj_align;
> }
>
> poly_uint64 const_size;
> --
> 2.30.2
>
