[RS6000] pr65810, powerpc64 alignment of r2

This fixes a thinko in offsettable_ok_by_alignment.  It's not the
absolute placement that matters, but the toc-pointer relative offset.
So alignment of r2 also needs to be taken into account.

Bootstrapped and regression tested powerpc64-linux.  OK for mainline
and gcc-5 branch?  Without the dead code removal for the branch..

I also have a linker fix to align the toc pointer and gcc configury
changes to supply POWERPC64_TOC_POINTER_ALIGNMENT in the works.

	PR target/65810
	* config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define.
	(offsettable_ok_by_alignment): Return false if size exceeds
	POWERPC64_TOC_POINTER_ALIGNMENT.  Replace dead code with assertion.

On Mon, Apr 20, 2015 at 11:23:17PM +0930, Alan Modra wrote:
> This fixes a thinko in offsettable_ok_by_alignment.  It's not the
> absolute placement that matters, but the toc-pointer relative offset.
> So alignment of r2 also needs to be taken into account.
> 
> Bootstrapped and regression tested powerpc64-linux.  OK for mainline
> and gcc-5 branch?  Without the dead code removal for the branch..

Please wait until 5.1 is released on the gcc-5 branch.

	Jakub

On Mon, Apr 20, 2015 at 9:53 AM, Alan Modra <amodra@gmail.com> wrote:
> This fixes a thinko in offsettable_ok_by_alignment.  It's not the
> absolute placement that matters, but the toc-pointer relative offset.
> So alignment of r2 also needs to be taken into account.
>
> Bootstrapped and regression tested powerpc64-linux.  OK for mainline
> and gcc-5 branch?  Without the dead code removal for the branch..

See question below.

>
> I also have a linker fix to align the toc pointer and gcc configury
> changes to supply POWERPC64_TOC_POINTER_ALIGNMENT in the works.
>
>         PR target/65810
>         * config/rs6000/rs6000.c (POWERPC64_TOC_POINTER_ALIGNMENT): Define.
>         (offsettable_ok_by_alignment): Return false if size exceeds
>         POWERPC64_TOC_POINTER_ALIGNMENT.  Replace dead code with assertion.
>
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c  (revision 222227)
> +++ gcc/config/rs6000/rs6000.c  (working copy)
> @@ -6497,13 +6497,21 @@ virtual_stack_registers_memory_p (rtx op)
>  }
>
>  /* Return true if a MODE sized memory accesses to OP plus OFFSET
> -   is known to not straddle a 32k boundary.  */
> +   is known to not straddle a 32k boundary.  This function is used
> +   to determine whether -mcmodel=medium code can use TOC pointer
> +   relative addressing for OP.  This means the alignment of the TOC
> +   pointer must also be taken into account, and unfortunately that is
> +   only 8 bytes.  */
>
> +#ifndef POWERPC64_TOC_POINTER_ALIGNMENT
> +#define POWERPC64_TOC_POINTER_ALIGNMENT 8
> +#endif
> +
>  static bool
>  offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT offset,
>                              machine_mode mode)
>  {
> -  tree decl, type;
> +  tree decl;
>    unsigned HOST_WIDE_INT dsize, dalign, lsb, mask;
>
>    if (GET_CODE (op) != SYMBOL_REF)
> @@ -6510,6 +6518,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT
>      return false;
>
>    dsize = GET_MODE_SIZE (mode);
> +  if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT)
> +    return false;

Why do you immediately fail if the mode size is greater than the alignment?

You may want to sprinkle some comments that this is assuming natural
alignment (which is correct for the current ABI).  But the DECL could
be declared with less alignment.

VMX needs this sort of restriction, but VSX would function okay.

Or, does the generated code assume that the offsets from the TOC are
units of the DECL size and not the TOC size?

Thanks, David

>    decl = SYMBOL_REF_DECL (op);
>    if (!decl)
>      {
> @@ -6553,6 +6563,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT
>             return false;
>
>           dsize = tree_to_uhwi (DECL_SIZE_UNIT (decl));
> +         if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT)
> +           return false;
>           if (dsize > 32768)
>             return false;
>
> @@ -6560,32 +6572,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT
>         }
>      }
>    else
> -    {
> -      type = TREE_TYPE (decl);
> +    gcc_unreachable ();
>
> -      dalign = TYPE_ALIGN (type);
> -      if (CONSTANT_CLASS_P (decl))
> -       dalign = CONSTANT_ALIGNMENT (decl, dalign);
> -      else
> -       dalign = DATA_ALIGNMENT (decl, dalign);
> -
> -      if (dsize == 0)
> -       {
> -         /* BLKmode, check the entire object.  */
> -         if (TREE_CODE (decl) == STRING_CST)
> -           dsize = TREE_STRING_LENGTH (decl);
> -         else if (TYPE_SIZE_UNIT (type)
> -                  && tree_fits_uhwi_p (TYPE_SIZE_UNIT (type)))
> -           dsize = tree_to_uhwi (TYPE_SIZE_UNIT (type));
> -         else
> -           return false;
> -         if (dsize > 32768)
> -           return false;
> -
> -         return dalign / BITS_PER_UNIT >= dsize;
> -       }
> -    }
> -
>    /* Find how many bits of the alignment we know for this access.  */
>    mask = dalign / BITS_PER_UNIT - 1;
>    lsb = offset & -offset;
>
> --
> Alan Modra
> Australia Development Lab, IBM

On Mon, Apr 20, 2015 at 01:11:41PM -0400, David Edelsohn wrote:
> > @@ -6510,6 +6518,8 @@ offsettable_ok_by_alignment (rtx op, HOST_WIDE_INT
> >      return false;
> >
> >    dsize = GET_MODE_SIZE (mode);
> > +  if (dsize > POWERPC64_TOC_POINTER_ALIGNMENT)
> > +    return false;
> 
> Why do you immediately fail if the mode size is greater than the alignment?

The function is testing whether the address is offsettable within
mode.  To access components of the memory addressed, we might need to
tack on an additional offset up to dsize-1 to the toc-relative offset.
If the additional offset exceeds the known minimum toc pointer
alignment then we might go over a n*64k+32k boundary.  See
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65810#c4

Now, if you were really asking "Why didn't you defer this test and
instead take the lesser of the memory alignment and the toc pointer
alignment in order to cover

> But the DECL could be declared with less alignment.

this case?", then the answer would be that I'm a dummy.  I woke up
this morning with a nagging feeling that the patch wasn't quite
correct.

So, given that Jakub has nixed the patch for 5.1, I may as well not
rush this change.  I'll spend some time first testing the linker fix
and submit a better solution that has a configure test for
POWERPC64_TOC_POINTER_ALIGNMENT.

> VMX needs this sort of restriction, but VSX would function okay.
> 
> Or, does the generated code assume that the offsets from the TOC are
> units of the DECL size and not the TOC size?

I'm not sure what you're asking here.  Offsettable means we have
a D-form instruction.  As in the PR
   addis   r9,r2,-2
   lfd     f24,32760(r9)
   lfd     f25,-32768(r9)
is bad.  The patch I submitted changes this to
   addis   r9,r2,-2
   addi    r9,r9,32760
   lfd     f24,0(r9)
   lfd     f25,8(r9)