[patch] Respect known misaligned addresses in set_mem_attributes_minus_bitpos
Richard Guenther
richard.guenther@gmail.com
Thu Dec 30 16:57:00 GMT 2010
On Thu, Dec 30, 2010 at 1:22 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Richard Guenther wrote:
>
>> The correct way to fix this is IMHO to make set_mem_attributes_minus_bitpos
>> not get initial alignment from the mode (but assert the mem-attrs are not
>> set yet in that function). At least if I understand the problem correctly.
>>
>> Ulrich promised to do this a while back ...
>
> Sorry for the long delay; I finally found some time to work on this.
>
> The patch below changes set_mem_attributes_minus_bitpos to not rely
> on the implicit defaulting rules in the MEM_ accessor macros. Instead,
> they are used only if memory attributes are in fact present. If not,
> deriving defaults for size and alignment from the MEM's mode are
> open-coded in this function.
>
> The overall behaviour should be identical to before, except for the one
> case we wanted to change: if set_mem_attributes_minus_bitpos is called
> with an expression (not a type) describing an object, the alignment is
> determined solely from that expression; default alignment of the mode
> is ignored, even on STRICT_ALIGNMENT targets.
>
> This fixes the problem I was originally seeing on a (modified) SPU target
> (and in fact generates fully identical code to what I was getting with my
> back-end work-around). Ramana, does this fix the ARM problem as well?
>
>
> There may be additional changes one could possibly do:
>
> - Is set_mem_attributes_minus_bitpos ever called on a MEM that already has
> attributes? If not, this set of defaults could go away ...
I think we shouldn't call it if there are already mem attributes, but
to be safe just
leave the code in for now (can you add a FIXME comment?).
> - Is there really any case where using the mode's *size* is necessary (i.e.
> where we don't already get the size from the type)?
I don't think so (but the function needs some overall cleanup anyway, your
changes look reasonable for stage3).
> - If we don't get an expression, but just a type, the rules when to use
> the type's alignment seem somewhat complex to me ... If this could
> be simplified so we can *always* use the type alignment, they all the
> STRICT_ALIGNMENT special case could maybe go there?
I think we should deprecate the case where we get a type in 't'. And indeed
if we get a type we should just use TYPE_ALIGN - mode alignment doesn't
honor alignment attributes, so you'd get wrong code again on strict align
targets.
> But I guess all this could be follow-on work.
Indeed.
> Tested on s390x-linux, powerpc64-linux and spu-elf with no regressions.
>
> OK for mainline?
Ok with some FIXME comments according to the issues you mention added.
Thanks,
Richard.
> Bye,
> Ulrich
>
>
> ChangeLog:
>
> * emit-rtl.c (set_mem_attributes_minus_bitpos): Explicitly derive
> default values from MEM mode if no memory attributes are present.
> Do not use mode alignment, even on STRICT_ALIGNMENT targets, when
> called with an expression (not a type).
>
> Index: gcc-head/gcc/emit-rtl.c
> ===================================================================
> --- gcc-head.orig/gcc/emit-rtl.c
> +++ gcc-head/gcc/emit-rtl.c
> @@ -1540,11 +1540,11 @@ void
> set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
> HOST_WIDE_INT bitpos)
> {
> - alias_set_type alias = MEM_ALIAS_SET (ref);
> - tree expr = MEM_EXPR (ref);
> - rtx offset = MEM_OFFSET (ref);
> - rtx size = MEM_SIZE (ref);
> - unsigned int align = MEM_ALIGN (ref);
> + alias_set_type alias;
> + tree expr = NULL;
> + rtx offset = NULL_RTX;
> + rtx size = NULL_RTX;
> + unsigned int align = BITS_PER_UNIT;
> HOST_WIDE_INT apply_bitpos = 0;
> tree type;
>
> @@ -1580,6 +1580,27 @@ set_mem_attributes_minus_bitpos (rtx ref
> && TREE_CODE (type) != COMPLEX_TYPE)
> MEM_SCALAR_P (ref) = 1;
>
> + /* Default values from pre-existing memory attributes if present. */
> + if (MEM_ATTRS (ref))
> + {
> + expr = MEM_EXPR (ref);
> + offset = MEM_OFFSET (ref);
> + size = MEM_SIZE (ref);
> + align = MEM_ALIGN (ref);
> + }
> +
> + /* Otherwise, default values from the mode of the MEM reference. */
> + else if (GET_MODE (ref) != BLKmode)
> + {
> + /* Respect mode size. */
> + size = GEN_INT (GET_MODE_SIZE (GET_MODE (ref)));
> +
> + /* Respect mode alignment for STRICT_ALIGNMENT targets if T is a type;
> + if T is an object, always compute the object alignment below. */
> + if (STRICT_ALIGNMENT && TYPE_P (t))
> + align = GET_MODE_ALIGNMENT (GET_MODE (ref));
> + }
> +
> /* We can set the alignment from the type if we are making an object,
> this is an INDIRECT_REF, or if TYPE_ALIGN_OK. */
> if (objectp || TREE_CODE (t) == INDIRECT_REF || TYPE_ALIGN_OK (type))
>
> --
> Dr. Ulrich Weigand
> GNU Toolchain for Linux on System z and Cell BE
> Ulrich.Weigand@de.ibm.com
>
More information about the Gcc-patches
mailing list