Re: Fix for IRA stack slot sharing codegen but on BYTES_BIG_ENDIAN targets

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Sat, Sep 6, 2008 at 11:07 PM, Jeff Law <law@redhat.com> wrote:
> There are several testsuite regressions when using IRA on the H8 port.
>
>
> The underlying problem is IRA will record the wrong address for a slot on a
> BYTES_BIG_ENDIAN target when two nonconflicting pseudos with different sizes
> are sharing a slot.
> So, assume we have two registers, A -- HImode and B -- SImode that do not
> conflict and can share a slot.  Assume we encounter A first.  We correctly
> allocate enough space for SImode, say at fp - 12.  But an endianness
> adjustment is performed in assign_stack_local so the returned address is
> actually fp - 10.  That's the address IRA records.  Later when we want the
> slot for B, we find the shared slot fp - 10.   Obviously storing B into fp
>  - 10 writes into part of the slot shared by A & B as well as the nearby
> memory locations fp -8 and fp - 7, which is very very very bad.
>
> The non-IRA codepath undoes the endianness adjustment before recording a
> slot for sharing purposes.  The IRA codepath seems to have part, but not all
> of the code necessary to undo the endianness adjustment.  This patch adds
> the missing code to the IRA path.
>
> This has not been checked in as it is awaiting review by a reload
> maintainer, global reviewer...

I think it would make sense to have an extra parameter to assign_stack_local
to return this unadjusted slot.  This would save us some code duplication at
least.

Richard.

>   * reload1.c (alter_reg): Undo the BYTES_BIG_ENDIAN correction performed
>   by assign_stack_local on the IRA path for stack slot sharing as well
>   as the non-IRA path.
>
>
>        * reload1.c (alter_reg): Undo the BYTE_BIG_ENDIAN correction
> performed
>        by assign_stack_local on the IRA path for stack slot sharing as well
>        as the non-IRA path.
>
> Index: reload1.c
> ===================================================================
> --- reload1.c   (revision 140073)
> +++ reload1.c   (working copy)
> @@ -2176,18 +2176,28 @@
>         inherent space, and no less total space, then the previous slot.  */
>       else if (from_reg == -1 || (! dont_share_p && flag_ira && optimize))
>        {
> +         rtx stack_slot;
>          alias_set_type alias_set = new_alias_set ();
>
>          /* No known place to spill from => no slot to reuse.  */
>          x = assign_stack_local (mode, total_size,
>                                  min_align > inherent_align
>                                  || total_size > inherent_size ? -1 : 0);
> +
> +         stack_slot = x;
> +
>          if (BYTES_BIG_ENDIAN)
>            /* Cancel the  big-endian correction done in assign_stack_local.
>               Get the address of the beginning of the slot.
>               This is so we can do a big-endian correction unconditionally
>               below.  */
>            adjust = inherent_size - total_size;
> +           if (adjust)
> +             stack_slot
> +               = adjust_address_nv (x, mode_for_size (total_size
> +                                                      * BITS_PER_UNIT,
> +                                                      MODE_INT, 1),
> +                                    adjust);
>
>          /* Nothing can alias this slot except this pseudo.  */
>          set_mem_alias_set (x, alias_set);
> @@ -2195,7 +2205,7 @@
>
>          if (! dont_share_p && flag_ira && optimize)
>            /* Inform IRA about allocation a new stack slot.  */
> -           ira_mark_new_stack_slot (x, i, total_size);
> +           ira_mark_new_stack_slot (stack_slot, i, total_size);
>        }
>
>       /* Reuse a stack slot if possible.  */
>
>