decl alignment not respected

This patch cures a problem with ICF of read-only variables at the
intersection of -fsection-anchors, -ftree-loop-vectorize, and targets
with alignment restrictions.  The testcase results in
/usr/local/powerpc64le-linux/bin/ld: pack.o: In function `main':
pack.c:(.text.startup+0xc): error: R_PPC64_TOC16_LO_DS not a multiple of 4
/usr/local/powerpc64le-linux/bin/ld: final link failed: Bad value
on powerpc64le-linux.

What happens is:
- "c" is referenced in a constructor, thus make_decl_rtl for "c",
- make_decl_rtl puts "c" in an anchor block (-fsection-anchors),
- anchor block contents can't move, so "c" alignment can't change by
  ipa_increase_alignment (-ftree-loop-vectorize),
- however "a" alignment can be increased,
- ICF aliases "a" to "c".
So we have a decl for "a" saying it is aligned to 128 bits, using mem
for "c" which is only 16 bit aligned.  The supposed increased
alignment causes "a" to be accessed as a 64-bit word, and the
powerpc64 backend to use a ds-form addressing mode.

Somewhere this chain of events needs to be broken.  It isn't possible
to stop ipa_increase_alignment changing "a", because at that stage
ICF aliases don't exist.  So it seemed to me that ICF needed to be
taught not to create a problematic alias.  Not being very familiar
with this code, I don't know if the following is the best place to
change, but sem_variable::merge does throw away aliases for quite a
lot of other reasons.  Another possibility is
sem_variable::equals_wpa, where there's a comment about DECL_ALIGN
being safe to merge "because we will always choose the largest
alignment out of all aliases".  Except with this testcase we don't
choose the largest alignment, and indeed can't (I think) due to "c"
being used in a constructor.

Bootstrapped and regression tested powerpc64le-linux.  OK to apply?

gcc/
	PR ipa/69990
	* ipa-icf.c (sem_variable::merge): Do not merge an alias with
	larger alignment.
gcc/testsuite/
	gcc.dg/pr69990.c: New.

On Tue, 1 Mar 2016, Alan Modra wrote:

> This patch cures a problem with ICF of read-only variables at the
> intersection of -fsection-anchors, -ftree-loop-vectorize, and targets
> with alignment restrictions.  The testcase results in
> /usr/local/powerpc64le-linux/bin/ld: pack.o: In function `main':
> pack.c:(.text.startup+0xc): error: R_PPC64_TOC16_LO_DS not a multiple of 4
> /usr/local/powerpc64le-linux/bin/ld: final link failed: Bad value
> on powerpc64le-linux.
> 
> What happens is:
> - "c" is referenced in a constructor, thus make_decl_rtl for "c",
> - make_decl_rtl puts "c" in an anchor block (-fsection-anchors),
> - anchor block contents can't move, so "c" alignment can't change by
>   ipa_increase_alignment (-ftree-loop-vectorize),
> - however "a" alignment can be increased,
> - ICF aliases "a" to "c".
> So we have a decl for "a" saying it is aligned to 128 bits, using mem
> for "c" which is only 16 bit aligned.  The supposed increased
> alignment causes "a" to be accessed as a 64-bit word, and the
> powerpc64 backend to use a ds-form addressing mode.
> 
> Somewhere this chain of events needs to be broken.  It isn't possible
> to stop ipa_increase_alignment changing "a", because at that stage
> ICF aliases don't exist.  So it seemed to me that ICF needed to be
> taught not to create a problematic alias.  Not being very familiar
> with this code, I don't know if the following is the best place to
> change, but sem_variable::merge does throw away aliases for quite a
> lot of other reasons.  Another possibility is
> sem_variable::equals_wpa, where there's a comment about DECL_ALIGN
> being safe to merge "because we will always choose the largest
> alignment out of all aliases".  Except with this testcase we don't
> choose the largest alignment, and indeed can't (I think) due to "c"
> being used in a constructor.
> 
> Bootstrapped and regression tested powerpc64le-linux.  OK to apply?

Ok.

Thanks,
Richard.

> gcc/
> 	PR ipa/69990
> 	* ipa-icf.c (sem_variable::merge): Do not merge an alias with
> 	larger alignment.
> gcc/testsuite/
> 	gcc.dg/pr69990.c: New.
> 
> diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
> index ef04c55..d82eb87 100644
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -2209,6 +2209,16 @@ sem_variable::merge (sem_item *alias_item)
>  		 "adress of original and alias may be compared.\n\n");
>        return false;
>      }
> +
> +  if (DECL_ALIGN (original->decl) < DECL_ALIGN (alias->decl))
> +    {
> +      if (dump_file)
> +	fprintf (dump_file, "Not unifying; "
> +		 "original and alias have incompatible alignments\n\n");
> +
> +      return false;
> +    }
> +
>    if (DECL_COMDAT_GROUP (original->decl) != DECL_COMDAT_GROUP (alias->decl))
>      {
>        if (dump_file)
> diff --git a/gcc/testsuite/gcc.dg/pr69990.c b/gcc/testsuite/gcc.dg/pr69990.c
> new file mode 100644
> index 0000000..efb835e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr69990.c
> @@ -0,0 +1,24 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target section_anchors } */
> +/* { dg-options "-O2 -fsection-anchors -ftree-loop-vectorize" } */
> +
> +#pragma pack(1)
> +struct S0 {
> +  volatile int f0:12;
> +} static a[] = {{15}}, c[] = {{15}};
> +
> +struct S0 b[] = {{7}};
> +
> +int __attribute__ ((noinline, noclone))
> +ok (int a, int b, int c)
> +{
> +  return a == 15 && b == 7 && c == 15 ? 0 : 1;
> +}
> +
> +int
> +main (void)
> +{
> +  struct S0 *f[] = { c, b };
> +
> +  return ok (a[0].f0, b[0].f0, f[0]->f0);
> +}
> 
>