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);
> +}
>
>
--
Richard Biener <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
